This is an archive of the discontinued LLVM Phabricator instance.

[X86] Don't fold negative offset into 32-bit absolute address (e.g. movl $foo-1, %eax)
ClosedPublic

Authored by MaskRay on Dec 29 2020, 9:42 PM.

Details

Summary

When building abseil-cpp bin/absl_hash_test with Clang in -fno-pic
mode, an instruction like movl $foo-2147483648, $eax may be produced
(subtracting a number from the address of a static variable). If foo's
address is smaller than 2147483648, GNU ld/gold/LLD will error because
R_X86_64_32 cannot represent a negative value.

using absl::Hash;
struct NoOp {
  template < typename HashCode >
  friend HashCode AbslHashValue(HashCode , NoOp );
};
template <typename> class HashIntTest : public testing::Test {};
TYPED_TEST_SUITE_P(HashIntTest);
TYPED_TEST_P(HashIntTest, BasicUsage) {
  if (std::numeric_limits< TypeParam >::min )
    EXPECT_NE(Hash< NoOp >()({}),
              Hash< TypeParam >()(std::numeric_limits< TypeParam >::min()));
}
REGISTER_TYPED_TEST_CASE_P(HashIntTest, BasicUsage);
using IntTypes = testing::Types< int32_t>;
INSTANTIATE_TYPED_TEST_CASE_P(My, HashIntTest, IntTypes);

ld: error: hash_test.cc:(function (anonymous namespace)::gtest_suite_HashIntTest_::BasicUsage<int>::TestBody(): .text+0x4E472): relocation R_X86_64_32 out of range: 18446744071564237392 is not in [0, 4294967295]; references absl::hash_internal::HashState::kSeed

Actually any negative offset is not allowed because the symbol address
can be zero (e.g. set by -Wl,--defsym=foo=0). So disallow such folding.

Diff Detail

Event Timeline

MaskRay created this revision.Dec 29 2020, 9:42 PM
MaskRay requested review of this revision.Dec 29 2020, 9:42 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 29 2020, 9:42 PM
pengfei added inline comments.Dec 29 2020, 11:46 PM
llvm/lib/Target/X86/X86ISelLowering.cpp
19110–19111

Maybe just need to add Offset >=0 && here?

MaskRay updated this revision to Diff 314127.Dec 30 2020, 9:24 AM
MaskRay edited the summary of this revision. (Show Details)

Simplify

MaskRay marked an inline comment as done.Dec 30 2020, 9:24 AM
pengfei accepted this revision.Dec 30 2020, 5:17 PM

LGTM.

This revision is now accepted and ready to land.Dec 30 2020, 5:17 PM
MaskRay updated this revision to Diff 314160.Dec 30 2020, 6:46 PM
MaskRay edited the summary of this revision. (Show Details)

Improve description

This revision was landed with ongoing or failed builds.Dec 30 2020, 6:47 PM
This revision was automatically updated to reflect the committed changes.