Skip to content

Commit 23faa81

Browse files
authored
[SHT_LLVM_BB_ADDR_MAP] Avoids side-effects in addition since order is unspecified. (#79168)
Turns out the problem with #60013 is due to the fact that order of operation is unspecified in C++: https://en.cppreference.com/w/cpp/language/eval_order. A small example of where this manifests with MSVC can be seen here https://ooo.godbolt.org/z/bxqKeqzqn. This patch does the following: * Removes the addition operations where we sequence more than one side-effect based expression. * Removes test guards to now run on Windows
1 parent 3a92b20 commit 23faa81

File tree

9 files changed

+7
-41
lines changed

9 files changed

+7
-41
lines changed

llvm/lib/ObjectYAML/ELFEmitter.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1439,9 +1439,9 @@ void ELFState<ELFT>::writeSectionContent(
14391439
for (const ELFYAML::BBAddrMapEntry::BBEntry &BBE : *E.BBEntries) {
14401440
if (Section.Type == llvm::ELF::SHT_LLVM_BB_ADDR_MAP && E.Version > 1)
14411441
SHeader.sh_size += CBA.writeULEB128(BBE.ID);
1442-
SHeader.sh_size += CBA.writeULEB128(BBE.AddressOffset) +
1443-
CBA.writeULEB128(BBE.Size) +
1444-
CBA.writeULEB128(BBE.Metadata);
1442+
SHeader.sh_size += CBA.writeULEB128(BBE.AddressOffset);
1443+
SHeader.sh_size += CBA.writeULEB128(BBE.Size);
1444+
SHeader.sh_size += CBA.writeULEB128(BBE.Metadata);
14451445
}
14461446
}
14471447

@@ -1469,8 +1469,10 @@ void ELFState<ELFT>::writeSectionContent(
14691469
SHeader.sh_size += CBA.writeULEB128(*PGOBBE.BBFreq);
14701470
if (PGOBBE.Successors) {
14711471
SHeader.sh_size += CBA.writeULEB128(PGOBBE.Successors->size());
1472-
for (const auto &[ID, BrProb] : *PGOBBE.Successors)
1473-
SHeader.sh_size += CBA.writeULEB128(ID) + CBA.writeULEB128(BrProb);
1472+
for (const auto &[ID, BrProb] : *PGOBBE.Successors) {
1473+
SHeader.sh_size += CBA.writeULEB128(ID);
1474+
SHeader.sh_size += CBA.writeULEB128(BrProb);
1475+
}
14741476
}
14751477
}
14761478
}

llvm/test/tools/llvm-objdump/X86/elf-bbaddrmap-disassemble-symbolize-operands.yaml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
## Test that in the presence of SHT_LLVM_BB_ADDR_MAP sections,
22
## --symbolize-operands can display <BB*> labels.
33

4-
## Fails on windows (https://github.com/llvm/llvm-project/issues/60013).
5-
# UNSUPPORTED: system-windows
6-
74
## Executable object file.
85
# RUN: yaml2obj --docnum=1 -DFOO_ADDR=0x4000 -DBAR_ADDR=0x5000 %s -o %t1
96
# RUN: llvm-objdump %t1 -d --symbolize-operands -M intel --no-show-raw-insn --no-leading-addr | \

llvm/test/tools/llvm-objdump/X86/elf-bbaddrmap-symbolize-relocatable.yaml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,6 @@
22
## --symbolize-operands can display <BB*> labels properly in a relocatable
33
## object file.
44

5-
## Fails on windows (https://github.com/llvm/llvm-project/issues/60013).
6-
# UNSUPPORTED: system-windows
7-
85
## Relocatable Object file.
96
# RUN: yaml2obj %s -o %t1
107
# RUN: llvm-objdump %t1 -d --symbolize-operands -M att --no-show-raw-insn --no-leading-addr | \

llvm/test/tools/llvm-objdump/X86/elf-pgoanalysismap.yaml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,6 @@
22
## contain PGO data, --symbolize-operands is able to label the basic blocks
33
## correctly.
44

5-
## Fails on windows (https://github.com/llvm/llvm-project/issues/60013).
6-
# UNSUPPORTED: system-windows
7-
85
## Check the case where we only have entry counts.
96

107
# RUN: yaml2obj --docnum=1 %s -o %t1

llvm/test/tools/llvm-readobj/ELF/bb-addr-map-relocatable.test

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
## This test checks how we handle the --bb-addr-map option on relocatable
22
## object files.
33

4-
## Fails on windows (https://github.com/llvm/llvm-project/issues/60013).
5-
# UNSUPPORTED: system-windows
6-
74
# RUN: yaml2obj %s -o %t1.o
85
# RUN: llvm-readobj %t1.o --bb-addr-map | FileCheck %s
96

llvm/test/tools/llvm-readobj/ELF/bb-addr-map.test

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,5 @@
11
## This test checks how we handle the --bb-addr-map option.
22

3-
## Fails on windows (https://github.com/llvm/llvm-project/issues/60013).
4-
# UNSUPPORTED: system-windows
5-
63
## Check 64-bit:
74
# RUN: yaml2obj --docnum=1 %s -DBITS=64 -DADDR=0x999999999 -o %t1.x64.o
85
# RUN: llvm-readobj %t1.x64.o --bb-addr-map 2>&1 | FileCheck %s -DADDR=0x999999999 -DFILE=%t1.x64.o --check-prefix=CHECK

llvm/test/tools/obj2yaml/ELF/bb-addr-map.yaml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,5 @@
11
## Check how obj2yaml produces YAML .llvm_bb_addr_map descriptions.
22

3-
## Fails on windows (https://github.com/llvm/llvm-project/issues/60013).
4-
# UNSUPPORTED: system-windows
5-
63
## Check that obj2yaml uses the "Entries" tag to describe an .llvm_bb_addr_map section.
74

85
# RUN: yaml2obj --docnum=1 %s -o %t1

llvm/test/tools/yaml2obj/ELF/bb-addr-map.yaml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,5 @@
11
## Check how yaml2obj produces .llvm_bb_addr_map sections.
22

3-
## Fails on windows (https://github.com/llvm/llvm-project/issues/60013).
4-
# UNSUPPORTED: system-windows
5-
63
# RUN: yaml2obj --docnum=1 %s -o %t1
74
# RUN: llvm-readobj --sections --section-data %t1 | FileCheck %s
85

llvm/unittests/Object/ELFObjectFileTest.cpp

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,6 @@
2121
using namespace llvm;
2222
using namespace llvm::object;
2323

24-
// Used to skip LLVM_BB_ADDR_MAP tests on windows platforms due to
25-
// https://github.com/llvm/llvm-project/issues/60013.
26-
bool IsHostWindows() {
27-
Triple Host(Triple::normalize(sys::getProcessTriple()));
28-
return Host.isOSWindows();
29-
}
30-
3124
namespace {
3225

3326
// A struct to initialize a buffer to represent an ELF object file.
@@ -508,8 +501,6 @@ TEST(ELFObjectFileTest, InvalidSymbolTest) {
508501

509502
// Tests for error paths of the ELFFile::decodeBBAddrMap API.
510503
TEST(ELFObjectFileTest, InvalidDecodeBBAddrMap) {
511-
if (IsHostWindows())
512-
GTEST_SKIP();
513504
StringRef CommonYamlString(R"(
514505
--- !ELF
515506
FileHeader:
@@ -656,8 +647,6 @@ TEST(ELFObjectFileTest, InvalidDecodeBBAddrMap) {
656647

657648
// Test for the ELFObjectFile::readBBAddrMap API.
658649
TEST(ELFObjectFileTest, ReadBBAddrMap) {
659-
if (IsHostWindows())
660-
GTEST_SKIP();
661650
StringRef CommonYamlString(R"(
662651
--- !ELF
663652
FileHeader:
@@ -816,8 +805,6 @@ TEST(ELFObjectFileTest, ReadBBAddrMap) {
816805
// Tests for error paths of the ELFFile::decodeBBAddrMap with PGOAnalysisMap
817806
// API.
818807
TEST(ELFObjectFileTest, InvalidDecodePGOAnalysisMap) {
819-
if (IsHostWindows())
820-
GTEST_SKIP();
821808
StringRef CommonYamlString(R"(
822809
--- !ELF
823810
FileHeader:
@@ -948,8 +935,6 @@ TEST(ELFObjectFileTest, InvalidDecodePGOAnalysisMap) {
948935

949936
// Test for the ELFObjectFile::readBBAddrMap API with PGOAnalysisMap.
950937
TEST(ELFObjectFileTest, ReadPGOAnalysisMap) {
951-
if (IsHostWindows())
952-
GTEST_SKIP();
953938
StringRef CommonYamlString(R"(
954939
--- !ELF
955940
FileHeader:

0 commit comments

Comments
 (0)