This is an archive of the discontinued LLVM Phabricator instance.

Honor -fuse-init-array when os is not specified on x86
ClosedPublic

Authored by kamleshbhalui on Dec 11 2019, 7:23 AM.

Details

Summary

Currently -fuse-init-array option is not effective when target triple does not specify os, on x86,x86_64.
i.e.
$clang -target i386 -fuse-init-array test.c -S // -fuse-init-array is not honored.

$clang -target i386-linux -fuse-init-array test.c -S // -fuse-init-array is honored.

This patch fixes first case.
And does cleanup.

Diff Detail

Event Timeline

kamleshbhalui created this revision.Dec 11 2019, 7:23 AM
kamleshbhalui retitled this revision from Accept -fuse-init-array when os is not specified on x86 to Honor -fuse-init-array when os is not specified on x86.Dec 11 2019, 8:24 AM
kamleshbhalui added a reviewer: fhahn.

Is it possible to add a test for this?

@craig.topper Thanks for reviewing.
I have added a test.

rnk added a comment.Dec 11 2019, 11:30 AM

It looks to me as if essentially all ELF targets want to honor the UseInitArray target option:

llvm/lib/Target/AArch64/AArch64TargetObjectFile.cpp:  InitializeELF(TM.Options.UseInitArray);
llvm/lib/Target/Hexagon/HexagonTargetObjectFile.cpp:  InitializeELF(TM.Options.UseInitArray);
llvm/lib/Target/Lanai/LanaiTargetObjectFile.cpp:  InitializeELF(TM.Options.UseInitArray);
llvm/lib/Target/Mips/MipsTargetObjectFile.cpp:  InitializeELF(TM.Options.UseInitArray);
llvm/lib/Target/PowerPC/PPCTargetObjectFile.cpp:  InitializeELF(TM.Options.UseInitArray);
llvm/lib/Target/RISCV/RISCVTargetObjectFile.cpp:  InitializeELF(TM.Options.UseInitArray);
llvm/lib/Target/Sparc/SparcTargetObjectFile.cpp:  InitializeELF(TM.Options.UseInitArray);
llvm/lib/Target/X86/X86TargetObjectFile.cpp:  InitializeELF(TM.Options.UseInitArray);
llvm/lib/Target/X86/X86TargetObjectFile.cpp:  InitializeELF(TM.Options.UseInitArray);
llvm/lib/Target/X86/X86TargetObjectFile.cpp:  InitializeELF(TM.Options.UseInitArray);
llvm/lib/Target/X86/X86TargetObjectFile.cpp:  InitializeELF(TM.Options.UseInitArray);

I think the code would be cleaner if we:

  • hoisted that setting up into some shared base class
  • OSs that insist on using ctors and not honoring this flag should be specialized to disable it (maybe NetBSD from your patch?)

What's needed here to prevent this type of problem in the future is a cleanup to reduce needlessly duplicated functionality, not yet another special case for OS-less triples.

llvm/lib/Target/X86/X86TargetObjectFile.h
52

I don't see how this differs in behavior in a meaningful way from X86ELFTargetObjectFile. Can you add the override there without breaking too much?

craig.topper added inline comments.Dec 11 2019, 11:43 AM
llvm/test/CodeGen/X86/init-array.ll
1

Would adding this RUN line to the existing constructors.ll be enough to test this?

kamleshbhalui edited the summary of this revision. (Show Details)

moved common code outside of target.
and added and modified checks.

bcain added a subscriber: bcain.Dec 12 2019, 7:59 AM
rnk accepted this revision.Dec 12 2019, 10:43 AM

So, this will lead to llc using .init_array by default, because llc doesn't know which ELF targets default to ctors, but clang does. See @MaskRay's clang change. I'm OK with that.

This looks good to me, but I would like to get a second approval from any of the other listed reviewers.

llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
93 ↗(On Diff #233512)

This whitespace change seems unnecessary, this is not a function doc comment, there should be a blank line here.

llvm/lib/Target/X86/X86TargetObjectFile.h
52

Oh, wow, what a simplification. :)

This revision is now accepted and ready to land.Dec 12 2019, 10:43 AM

Formatted and rebased.

This revision was automatically updated to reflect the committed changes.
hctim added a subscriber: hctim.Dec 17 2019, 7:38 AM

Reverted in 2423774cc2a040d34d32aacaf261805cb195ebd2 due to breaking the sanitizer buildbots: http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/37410

Please see https://github.com/google/sanitizers/wiki/SanitizerBotReproduceBuild for more information about how to repro our buildbots (MSan in particular is tricky).

-- Testing: 34896 tests, 64 workers --
Testing:  0.. 10.. 20.. 30
FAIL: LLVM :: CodeGen/MIR/X86/instr-symbols-and-mcsymbol-operands.mir (12314 of 34896)
******************** TEST 'LLVM :: CodeGen/MIR/X86/instr-symbols-and-mcsymbol-operands.mir' FAILED ********************
Script:
--
: 'RUN: at line 1';   /b/sanitizer-x86_64-linux-fast/build/llvm_build_ubsan/bin/llc -march=x86-64 -run-pass none -o - /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/test/CodeGen/MIR/X86/instr-symbols-and-mcsymbol-operands.mir | /b/sanitizer-x86_64-linux-fast/build/llvm_build_ubsan/bin/FileCheck /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/test/CodeGen/MIR/X86/instr-symbols-and-mcsymbol-operands.mir
--
Exit Code: 2

Command Output (stderr):
--
invalid subroutine type
!6 = distinct !DISubprogram(name: "test", scope: !7, file: !7, line: 4, type: !2, scopeLine: 4, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
!2 = !{}
warning: ignoring invalid debug info in /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/test/CodeGen/MIR/X86/instr-symbols-and-mcsymbol-operands.mir
/b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/include/llvm/MC/MCObjectFileInfo.h:399:50: runtime error: load of value 2037605229, which is not a valid value for type 'llvm::MCObjectFileInfo::Environment'
    #0 0x265ffd6 in getObjectFileType /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/include/llvm/MC/MCObjectFileInfo.h:399:50
    #1 0x265ffd6 in llvm::MCContext::createSymbolImpl(llvm::StringMapEntry<bool> const*, bool) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/MC/MCContext.cpp:162:19
    #2 0x265f927 in llvm::MCContext::createSymbol(llvm::StringRef, bool, bool) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/MC/MCContext.cpp:205:14
    #3 0x265f4bc in llvm::MCContext::getOrCreateSymbol(llvm::Twine const&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/MC/MCContext.cpp:138:11
    #4 0x26fdced in (anonymous namespace)::MIParser::getOrCreateMCSymbol(llvm::StringRef) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/CodeGen/MIRParser/MIParser.cpp:3058:26
    #5 0x26f89bd in (anonymous namespace)::MIParser::parsePreOrPostInstrSymbol(llvm::MCSymbol*&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/CodeGen/MIRParser/MIParser.cpp:2956:12
    #6 0x26f4d02 in (anonymous namespace)::MIParser::parse(llvm::MachineInstr*&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/CodeGen/MIRParser/MIParser.cpp:931:9
    #7 0x26f0a18 in parseBasicBlock /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/CodeGen/MIRParser/MIParser.cpp:814:9
    #8 0x26f0a18 in parseBasicBlocks /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/CodeGen/MIRParser/MIParser.cpp:876:9
    #9 0x26f0a18 in llvm::parseMachineInstructions(llvm::PerFunctionMIParsingState&, llvm::StringRef, llvm::SMDiagnostic&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/CodeGen/MIRParser/MIParser.cpp:3077:36
    #10 0x27030d1 in llvm::MIRParserImpl::initializeMachineFunction(llvm::yaml::MachineFunction const&, llvm::MachineFunction&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/CodeGen/MIRParser/MIRParser.cpp:451:7
    #11 0x27029fd in llvm::MIRParserImpl::parseMachineFunction(llvm::Module&, llvm::MachineModuleInfo&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/CodeGen/MIRParser/MIRParser.cpp:300:7
    #12 0x270268d in llvm::MIRParserImpl::parseMachineFunctions(llvm::Module&, llvm::MachineModuleInfo&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/CodeGen/MIRParser/MIRParser.cpp:251:9
    #13 0x862a74 in compileModule(char**, llvm::LLVMContext&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/tools/llc/llc.cpp:584:16
    #14 0x8600bb in main /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/tools/llc/llc.cpp:356:22
    #15 0x7fae45e532e0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202e0)
    #16 0x842f89 in _start (/b/sanitizer-x86_64-linux-fast/build/llvm_build_ubsan/bin/llc+0x842f89)

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/include/llvm/MC/MCObjectFileInfo.h:399:50 in 
FileCheck error: '-' is empty.
FileCheck command line:  /b/sanitizer-x86_64-linux-fast/build/llvm_build_ubsan/bin/FileCheck /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/test/CodeGen/MIR/X86/instr-symbols-and-mcsymbol-operands.mir

--

********************
Testing:  0.. 10.. 20.. 30.. 40.. 50
FAIL: LLVM :: CodeGen/X86/instr-symbols.mir (18443 of 34896)
******************** TEST 'LLVM :: CodeGen/X86/instr-symbols.mir' FAILED ********************
Script:
--
: 'RUN: at line 1';   /b/sanitizer-x86_64-linux-fast/build/llvm_build_ubsan/bin/llc -mtriple=x86_64-unknown-linux -start-before=x86-flags-copy-lowering -o - /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/test/CodeGen/X86/instr-symbols.mir | /b/sanitizer-x86_64-linux-fast/build/llvm_build_ubsan/bin/FileCheck /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/test/CodeGen/X86/instr-symbols.mir
--
Exit Code: 2

Command Output (stderr):
--
/b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/include/llvm/MC/MCObjectFileInfo.h:399:50: runtime error: load of value 175335265, which is not a valid value for type 'llvm::MCObjectFileInfo::Environment'
    #0 0x265ffd6 in getObjectFileType /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/include/llvm/MC/MCObjectFileInfo.h:399:50
    #1 0x265ffd6 in llvm::MCContext::createSymbolImpl(llvm::StringMapEntry<bool> const*, bool) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/MC/MCContext.cpp:162:19
    #2 0x265f927 in llvm::MCContext::createSymbol(llvm::StringRef, bool, bool) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/MC/MCContext.cpp:205:14
    #3 0x265f4bc in llvm::MCContext::getOrCreateSymbol(llvm::Twine const&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/MC/MCContext.cpp:138:11
    #4 0x26fdced in (anonymous namespace)::MIParser::getOrCreateMCSymbol(llvm::StringRef) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/CodeGen/MIRParser/MIParser.cpp:3058:26
    #5 0x26f89bd in (anonymous namespace)::MIParser::parsePreOrPostInstrSymbol(llvm::MCSymbol*&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/CodeGen/MIRParser/MIParser.cpp:2956:12
    #6 0x26f4d02 in (anonymous namespace)::MIParser::parse(llvm::MachineInstr*&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/CodeGen/MIRParser/MIParser.cpp:931:9
    #7 0x26f0a18 in parseBasicBlock /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/CodeGen/MIRParser/MIParser.cpp:814:9
    #8 0x26f0a18 in parseBasicBlocks /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/CodeGen/MIRParser/MIParser.cpp:876:9
    #9 0x26f0a18 in llvm::parseMachineInstructions(llvm::PerFunctionMIParsingState&, llvm::StringRef, llvm::SMDiagnostic&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/CodeGen/MIRParser/MIParser.cpp:3077:36
    #10 0x27030d1 in llvm::MIRParserImpl::initializeMachineFunction(llvm::yaml::MachineFunction const&, llvm::MachineFunction&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/CodeGen/MIRParser/MIRParser.cpp:451:7
    #11 0x27029fd in llvm::MIRParserImpl::parseMachineFunction(llvm::Module&, llvm::MachineModuleInfo&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/CodeGen/MIRParser/MIRParser.cpp:300:7
    #12 0x270268d in llvm::MIRParserImpl::parseMachineFunctions(llvm::Module&, llvm::MachineModuleInfo&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/CodeGen/MIRParser/MIRParser.cpp:251:9
    #13 0x862a74 in compileModule(char**, llvm::LLVMContext&) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/tools/llc/llc.cpp:584:16
    #14 0x8600bb in main /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/tools/llc/llc.cpp:356:22
    #15 0x7f6c4b2bc2e0 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202e0)
    #16 0x842f89 in _start (/b/sanitizer-x86_64-linux-fast/build/llvm_build_ubsan/bin/llc+0x842f89)

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/include/llvm/MC/MCObjectFileInfo.h:399:50 in 
FileCheck error: '-' is empty.
FileCheck command line:  /b/sanitizer-x86_64-linux-fast/build/llvm_build_ubsan/bin/FileCheck /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/test/CodeGen/X86/instr-symbols.mir

--

********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 

Testing Time: 155.11s
********************
Failing Tests (2):
    LLVM :: CodeGen/MIR/X86/instr-symbols-and-mcsymbol-operands.mir
    LLVM :: CodeGen/X86/instr-symbols.mir

  Expected Passes    : 34301
  Expected Failures  : 149
  Unsupported Tests  : 444
  Unexpected Failures: 2
FAILED: test/CMakeFiles/check-llvm
rnk added a comment.Dec 22 2019, 8:24 PM

I was able to reproduce this, but I think the issue is pre-existing. It looks like llc never calls TargetObjectFileInfo::Initialize when doing MIR parsing, but it probably should. The constructor of MCObjectFileInfo leaves the Env uninitialized, because it doesn't have the triple yet. The easiest way to observe the failure is with ubsan, since MSan is difficult to set up. I don't plan to debug this any further.