[llvm] [SHT_LLVM_BB_ADDR_MAP] Avoids side-effects in addition since order is unspecified. (PR #79168)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 23 08:56:50 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-platform-windows
Author: Micah Weston (red1bluelost)
<details>
<summary>Changes</summary>
Turns out the problem with https://github.com/llvm/llvm-project/issues/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
---
Full diff: https://github.com/llvm/llvm-project/pull/79168.diff
9 Files Affected:
- (modified) llvm/lib/ObjectYAML/ELFEmitter.cpp (+7-5)
- (modified) llvm/test/tools/llvm-objdump/X86/elf-bbaddrmap-disassemble-symbolize-operands.yaml (-3)
- (modified) llvm/test/tools/llvm-objdump/X86/elf-bbaddrmap-symbolize-relocatable.yaml (-3)
- (modified) llvm/test/tools/llvm-objdump/X86/elf-pgoanalysismap.yaml (-3)
- (modified) llvm/test/tools/llvm-readobj/ELF/bb-addr-map-relocatable.test (-3)
- (modified) llvm/test/tools/llvm-readobj/ELF/bb-addr-map.test (-3)
- (modified) llvm/test/tools/obj2yaml/ELF/bb-addr-map.yaml (-3)
- (modified) llvm/test/tools/yaml2obj/ELF/bb-addr-map.yaml (-3)
- (modified) llvm/unittests/Object/ELFObjectFileTest.cpp (-15)
``````````diff
diff --git a/llvm/lib/ObjectYAML/ELFEmitter.cpp b/llvm/lib/ObjectYAML/ELFEmitter.cpp
index 94b0529f761052b..b044d502c997564 100644
--- a/llvm/lib/ObjectYAML/ELFEmitter.cpp
+++ b/llvm/lib/ObjectYAML/ELFEmitter.cpp
@@ -1439,9 +1439,9 @@ void ELFState<ELFT>::writeSectionContent(
for (const ELFYAML::BBAddrMapEntry::BBEntry &BBE : *E.BBEntries) {
if (Section.Type == llvm::ELF::SHT_LLVM_BB_ADDR_MAP && E.Version > 1)
SHeader.sh_size += CBA.writeULEB128(BBE.ID);
- SHeader.sh_size += CBA.writeULEB128(BBE.AddressOffset) +
- CBA.writeULEB128(BBE.Size) +
- CBA.writeULEB128(BBE.Metadata);
+ SHeader.sh_size += CBA.writeULEB128(BBE.AddressOffset);
+ SHeader.sh_size += CBA.writeULEB128(BBE.Size);
+ SHeader.sh_size += CBA.writeULEB128(BBE.Metadata);
}
}
@@ -1469,8 +1469,10 @@ void ELFState<ELFT>::writeSectionContent(
SHeader.sh_size += CBA.writeULEB128(*PGOBBE.BBFreq);
if (PGOBBE.Successors) {
SHeader.sh_size += CBA.writeULEB128(PGOBBE.Successors->size());
- for (const auto &[ID, BrProb] : *PGOBBE.Successors)
- SHeader.sh_size += CBA.writeULEB128(ID) + CBA.writeULEB128(BrProb);
+ for (const auto &[ID, BrProb] : *PGOBBE.Successors) {
+ SHeader.sh_size += CBA.writeULEB128(ID);
+ SHeader.sh_size += CBA.writeULEB128(BrProb);
+ }
}
}
}
diff --git a/llvm/test/tools/llvm-objdump/X86/elf-bbaddrmap-disassemble-symbolize-operands.yaml b/llvm/test/tools/llvm-objdump/X86/elf-bbaddrmap-disassemble-symbolize-operands.yaml
index 2e9eb9af35615aa..6864932411aec21 100644
--- a/llvm/test/tools/llvm-objdump/X86/elf-bbaddrmap-disassemble-symbolize-operands.yaml
+++ b/llvm/test/tools/llvm-objdump/X86/elf-bbaddrmap-disassemble-symbolize-operands.yaml
@@ -1,9 +1,6 @@
## Test that in the presence of SHT_LLVM_BB_ADDR_MAP sections,
## --symbolize-operands can display <BB*> labels.
-## Fails on windows (https://github.com/llvm/llvm-project/issues/60013).
-# UNSUPPORTED: system-windows
-
## Executable object file.
# RUN: yaml2obj --docnum=1 -DFOO_ADDR=0x4000 -DBAR_ADDR=0x5000 %s -o %t1
# RUN: llvm-objdump %t1 -d --symbolize-operands -M intel --no-show-raw-insn --no-leading-addr | \
diff --git a/llvm/test/tools/llvm-objdump/X86/elf-bbaddrmap-symbolize-relocatable.yaml b/llvm/test/tools/llvm-objdump/X86/elf-bbaddrmap-symbolize-relocatable.yaml
index d5a618467bd2c99..f796963481cd44c 100644
--- a/llvm/test/tools/llvm-objdump/X86/elf-bbaddrmap-symbolize-relocatable.yaml
+++ b/llvm/test/tools/llvm-objdump/X86/elf-bbaddrmap-symbolize-relocatable.yaml
@@ -2,9 +2,6 @@
## --symbolize-operands can display <BB*> labels properly in a relocatable
## object file.
-## Fails on windows (https://github.com/llvm/llvm-project/issues/60013).
-# UNSUPPORTED: system-windows
-
## Relocatable Object file.
# RUN: yaml2obj %s -o %t1
# RUN: llvm-objdump %t1 -d --symbolize-operands -M att --no-show-raw-insn --no-leading-addr | \
diff --git a/llvm/test/tools/llvm-objdump/X86/elf-pgoanalysismap.yaml b/llvm/test/tools/llvm-objdump/X86/elf-pgoanalysismap.yaml
index a3dc0a7b6425ce3..c4bf443f920a283 100644
--- a/llvm/test/tools/llvm-objdump/X86/elf-pgoanalysismap.yaml
+++ b/llvm/test/tools/llvm-objdump/X86/elf-pgoanalysismap.yaml
@@ -2,9 +2,6 @@
## contain PGO data, --symbolize-operands is able to label the basic blocks
## correctly.
-## Fails on windows (https://github.com/llvm/llvm-project/issues/60013).
-# UNSUPPORTED: system-windows
-
## Check the case where we only have entry counts.
# RUN: yaml2obj --docnum=1 %s -o %t1
diff --git a/llvm/test/tools/llvm-readobj/ELF/bb-addr-map-relocatable.test b/llvm/test/tools/llvm-readobj/ELF/bb-addr-map-relocatable.test
index 6ac27941853fed6..e6b6cc344a8e7f2 100644
--- a/llvm/test/tools/llvm-readobj/ELF/bb-addr-map-relocatable.test
+++ b/llvm/test/tools/llvm-readobj/ELF/bb-addr-map-relocatable.test
@@ -1,9 +1,6 @@
## This test checks how we handle the --bb-addr-map option on relocatable
## object files.
-## Fails on windows (https://github.com/llvm/llvm-project/issues/60013).
-# UNSUPPORTED: system-windows
-
# RUN: yaml2obj %s -o %t1.o
# RUN: llvm-readobj %t1.o --bb-addr-map | FileCheck %s
diff --git a/llvm/test/tools/llvm-readobj/ELF/bb-addr-map.test b/llvm/test/tools/llvm-readobj/ELF/bb-addr-map.test
index 92805701751078f..0593f04d6e30167 100644
--- a/llvm/test/tools/llvm-readobj/ELF/bb-addr-map.test
+++ b/llvm/test/tools/llvm-readobj/ELF/bb-addr-map.test
@@ -1,8 +1,5 @@
## This test checks how we handle the --bb-addr-map option.
-## Fails on windows (https://github.com/llvm/llvm-project/issues/60013).
-# UNSUPPORTED: system-windows
-
## Check 64-bit:
# RUN: yaml2obj --docnum=1 %s -DBITS=64 -DADDR=0x999999999 -o %t1.x64.o
# RUN: llvm-readobj %t1.x64.o --bb-addr-map 2>&1 | FileCheck %s -DADDR=0x999999999 -DFILE=%t1.x64.o --check-prefix=CHECK
diff --git a/llvm/test/tools/obj2yaml/ELF/bb-addr-map.yaml b/llvm/test/tools/obj2yaml/ELF/bb-addr-map.yaml
index eceb42f6598f0fd..629c29e202aebda 100644
--- a/llvm/test/tools/obj2yaml/ELF/bb-addr-map.yaml
+++ b/llvm/test/tools/obj2yaml/ELF/bb-addr-map.yaml
@@ -1,8 +1,5 @@
## Check how obj2yaml produces YAML .llvm_bb_addr_map descriptions.
-## Fails on windows (https://github.com/llvm/llvm-project/issues/60013).
-# UNSUPPORTED: system-windows
-
## Check that obj2yaml uses the "Entries" tag to describe an .llvm_bb_addr_map section.
# RUN: yaml2obj --docnum=1 %s -o %t1
diff --git a/llvm/test/tools/yaml2obj/ELF/bb-addr-map.yaml b/llvm/test/tools/yaml2obj/ELF/bb-addr-map.yaml
index 73c9168084170b5..2086dc53208b06e 100644
--- a/llvm/test/tools/yaml2obj/ELF/bb-addr-map.yaml
+++ b/llvm/test/tools/yaml2obj/ELF/bb-addr-map.yaml
@@ -1,8 +1,5 @@
## Check how yaml2obj produces .llvm_bb_addr_map sections.
-## Fails on windows (https://github.com/llvm/llvm-project/issues/60013).
-# UNSUPPORTED: system-windows
-
# RUN: yaml2obj --docnum=1 %s -o %t1
# RUN: llvm-readobj --sections --section-data %t1 | FileCheck %s
diff --git a/llvm/unittests/Object/ELFObjectFileTest.cpp b/llvm/unittests/Object/ELFObjectFileTest.cpp
index b344e8abbccfc06..c539d79ac14e8a6 100644
--- a/llvm/unittests/Object/ELFObjectFileTest.cpp
+++ b/llvm/unittests/Object/ELFObjectFileTest.cpp
@@ -21,13 +21,6 @@
using namespace llvm;
using namespace llvm::object;
-// Used to skip LLVM_BB_ADDR_MAP tests on windows platforms due to
-// https://github.com/llvm/llvm-project/issues/60013.
-bool IsHostWindows() {
- Triple Host(Triple::normalize(sys::getProcessTriple()));
- return Host.isOSWindows();
-}
-
namespace {
// A struct to initialize a buffer to represent an ELF object file.
@@ -508,8 +501,6 @@ TEST(ELFObjectFileTest, InvalidSymbolTest) {
// Tests for error paths of the ELFFile::decodeBBAddrMap API.
TEST(ELFObjectFileTest, InvalidDecodeBBAddrMap) {
- if (IsHostWindows())
- GTEST_SKIP();
StringRef CommonYamlString(R"(
--- !ELF
FileHeader:
@@ -656,8 +647,6 @@ TEST(ELFObjectFileTest, InvalidDecodeBBAddrMap) {
// Test for the ELFObjectFile::readBBAddrMap API.
TEST(ELFObjectFileTest, ReadBBAddrMap) {
- if (IsHostWindows())
- GTEST_SKIP();
StringRef CommonYamlString(R"(
--- !ELF
FileHeader:
@@ -816,8 +805,6 @@ TEST(ELFObjectFileTest, ReadBBAddrMap) {
// Tests for error paths of the ELFFile::decodeBBAddrMap with PGOAnalysisMap
// API.
TEST(ELFObjectFileTest, InvalidDecodePGOAnalysisMap) {
- if (IsHostWindows())
- GTEST_SKIP();
StringRef CommonYamlString(R"(
--- !ELF
FileHeader:
@@ -948,8 +935,6 @@ TEST(ELFObjectFileTest, InvalidDecodePGOAnalysisMap) {
// Test for the ELFObjectFile::readBBAddrMap API with PGOAnalysisMap.
TEST(ELFObjectFileTest, ReadPGOAnalysisMap) {
- if (IsHostWindows())
- GTEST_SKIP();
StringRef CommonYamlString(R"(
--- !ELF
FileHeader:
``````````
</details>
https://github.com/llvm/llvm-project/pull/79168
More information about the llvm-commits
mailing list