Add debug support for set types

[demoitem]

Summary

This commit adds debugging support for set types defined in languages such as pascal and modula-2

[dblaikie]

Seems reasonable to me - thanks!

[aprantl]

In your example you're encoding set as a derived type over an enum. That seems like a fine approach to me, but I think the interface should enforce this. Perhaps the Verifier should reject set derived typed from anything but an enum and the DIBuilder should only accept DICompositeTypes as the base type? Are there non-enum set types that you also want to encode?
Does this also need dwarfdump support, or does this work already?

[demoitem]

Added extra Verifier checks to ensure the base type of a set is either an Enumeration or a Base type with integer encoding.
I will extend this check when I land a patch to extend subranges since a set can have a subrange as a basetype.
The llvm-dwarfdump tool outputs set debugging information without any enhancements.
I do not have commit access, so I wonder if someone could commit these changes if acceptable?

[demoitem]

Could someone please commit this patch if acceptable.

[aprantl]

Looks like you accidentally just updated the diff on top of your previous patch?
Is the Verifier change being tested?

[demoitem]

I'm new to this, my first contribution, so bear with me.

On my local branch I had 2 commits
and I used the following to get a patch (following the docs)

git show HEAD -U999999 > set.patch

The second patch I uploaded seemed to only have the changes from the second commit - which I thought was okay.

Can you advise? Is each patch supposed to contain the complete set of commits for the changed code and get a new Differential number?

I am slightly confused.

Also, regards the Verifier changes, I have only tested them locally but I guess I should produce a test file. So I will get started on that.

Thanks

[djtodoro]

In D76115#2001144, @demoitem wrote:

I'm new to this, my first contribution, so bear with me.

On my local branch I had 2 commits
and I used the following to get a patch (following the docs)

git show HEAD -U999999 > set.patch

The HEAD refers to the SHA of the specific commit.

The second patch I uploaded seemed to only have the changes from the second commit - which I thought was okay.

Can you advise? Is each patch supposed to contain the complete set of commits for the changed code and get a new Differential number?

Yes. If the patches are logically different it makes sense to make separate changes here. If the diffs are connected (they represent the same functionality/feature/intention), it makes sense to make a stack of the patches (Edit Related Revision -> Edit Parent/Child rev).

So, if your local repo looks as:

commit 878787...
Author: ...
Date:   ...

    [LLVM] ..

commit 989898...
Author: ...
Date:   ...

    [Clang] ..

you can create the patches for the Phabricator as:

git show 989898... -U999999 > name_of_the_change1.patch
git show 878787... -U999999 > name_of_the_change2.patch

I am slightly confused.

Also, regards the Verifier changes, I have only tested them locally but I guess I should produce a test file. So I will get started on that.

Thanks

[demoitem]

Added a new test

[demoitem]

I know this change is old but I'd like to get it committed if possible. (Got waylaid by the pandemic) Or maybe abandon it and resubmit?
Anyway, are there any reviewers out there?

[aprantl]

I think this looks mostly good, but it looks like it's missing a unittest for the DIBuilder function that you added.

[demoitem]

You wouldn't happen to know a similar unit test that I could base it on?

I checked out the unit test directory and could not see anything useful.

[aprantl]

In D76115#2537948, @demoitem wrote:

You wouldn't happen to know a similar unit test that I could base it on?

I checked out the unit test directory and could not see anything useful.

Did you look in unittests/IR/DebugInfoTest.cpp (e.g., at CreateFortranArrayTypeWithAttributes)?

[demoitem]

Added a test case for DIBuilder

[demoitem]

Great. I wonder if you might do the commit since I
don't have the rights.

[aprantl]

I was going to commit this, but I'm getting

FAIL: LLVM :: DebugInfo/Generic/set.ll (20019 of 43049)
******************** TEST 'LLVM :: DebugInfo/Generic/set.ll' FAILED ********************
Script:
--
: 'RUN: at line 3';   /Volumes/Data/llvm-project/_build.ninja.release/bin/llc -debugger-tune=gdb -dwarf-version=4 -filetype=obj -o /Volumes/Data/llvm-project/_build.ninja.release/test/DebugInfo/Generic/Output/set.ll.tmp.o < /Volumes/Data/llvm-project/llvm/test/DebugInfo/Generic/set.ll
: 'RUN: at line 4';   /Volumes/Data/llvm-project/_build.ninja.release/bin/llvm-dwarfdump -debug-info /Volumes/Data/llvm-project/_build.ninja.release/test/DebugInfo/Generic/Output/set.ll.tmp.o | /Volumes/Data/llvm-project/_build.ninja.release/bin/FileCheck /Volumes/Data/llvm-project/llvm/test/DebugInfo/Generic/set.ll --check-prefix=CHECK
--
Exit Code: 134

Command Output (stderr):
--
Wrong types for attribute: inalloca nest noalias nocapture nonnull readnone readonly byref(i64) byval(i64) preallocated(i64) sret(i64) align 1 dereferenceable(1) dereferenceable_or_null(1)
i8* (i64)* @Main_M3
LLVM ERROR: Broken module found, compilation aborted!
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace.
Stack dump:
0.	Program arguments: /Volumes/Data/llvm-project/_build.ninja.release/bin/llc -debugger-tune=gdb -dwarf-version=4 -filetype=obj -o /Volumes/Data/llvm-project/_build.ninja.release/test/DebugInfo/Generic/Output/set.ll.tmp.o
Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it):
0  llc                      0x00000001046e5d97 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) + 39
1  llc                      0x00000001046e4ef5 llvm::sys::RunSignalHandlers() + 85
2  llc                      0x00000001046e63d0 SignalHandler(int) + 272
3  libsystem_platform.dylib 0x00007fff203f3d7d _sigtramp + 29
4  libsystem_platform.dylib 000000000000000000 _sigtramp + 18446603339975148192
5  libsystem_c.dylib        0x00007fff20303411 abort + 120
6  llc                      0x0000000104635c05 llvm::report_fatal_error(llvm::Twine const&, bool) + 453
7  llc                      0x0000000104635a3b llvm::report_fatal_error(char const*, bool) + 43
8  llc                      0x0000000103ea388f llvm::UpgradeDebugInfo(llvm::Module&) + 207
9  llc                      0x000000010399b6ba llvm::LLParser::validateEndOfModule(bool) + 2570
10 llc                      0x000000010399a894 llvm::LLParser::Run(bool, llvm::function_ref<llvm::Optional<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > (llvm::StringRef)>) + 308
11 llc                      0x00000001039d8f62 parseAssemblyInto(llvm::MemoryBufferRef, llvm::Module*, llvm::ModuleSummaryIndex*, llvm::SMDiagnostic&, llvm::SlotMapping*, bool, llvm::function_ref<llvm::Optional<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > (llvm::StringRef)>) + 946
12 llc                      0x00000001039d9104 llvm::parseAssembly(llvm::MemoryBufferRef, llvm::SMDiagnostic&, llvm::LLVMContext&, llvm::SlotMapping*, llvm::function_ref<llvm::Optional<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > (llvm::StringRef)>) + 148
13 llc                      0x000000010401f6b5 llvm::parseIR(llvm::MemoryBufferRef, llvm::SMDiagnostic&, llvm::LLVMContext&, llvm::function_ref<llvm::Optional<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > (llvm::StringRef)>) + 709
14 llc                      0x000000010401f9ce llvm::parseIRFile(llvm::StringRef, llvm::SMDiagnostic&, llvm::LLVMContext&, llvm::function_ref<llvm::Optional<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > (llvm::StringRef)>) + 398
15 llc                      0x00000001025526e1 main + 3761
16 libdyld.dylib            0x00007fff203c9f3d start + 1
/Volumes/Data/llvm-project/_build.ninja.release/test/DebugInfo/Generic/Output/set.ll.script: line 2: 18343 Abort trap: 6           /Volumes/Data/llvm-project/_build.ninja.release/bin/llc -debugger-tune=gdb -dwarf-version=4 -filetype=obj -o /Volumes/Data/llvm-project/_build.ninja.release/test/DebugInfo/Generic/Output/set.ll.tmp.o < /Volumes/Data/llvm-project/llvm/test/DebugInfo/Generic/set.ll

[demoitem]

Oh dear.

Let me fix that.

[demoitem]

Had to fix a couple of tests and the dwarf definitions had moved.

[vitalybuka]

https://lab.llvm.org/buildbot/#/builders/5/builds/6180

FAIL: LLVM-Unit :: IR/./IRTests/DIBuilder.CreateSetType (70031 of 75187)
******************** TEST 'LLVM-Unit :: IR/./IRTests/DIBuilder.CreateSetType' FAILED ********************
Note: Google Test filter = DIBuilder.CreateSetType
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from DIBuilder
[ RUN      ] DIBuilder.CreateSetType
==45705==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x1fd3f80 in track /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/include/llvm/IR/Metadata.h:772:9
    #1 0x1fd3f80 in llvm::MDOperand::reset(llvm::Metadata*, llvm::Metadata*) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/include/llvm/IR/Metadata.h:767:5
    #2 0x1fcfc87 in setOperand /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/IR/Metadata.cpp:885:22
    #3 0x1fcfc87 in llvm::MDNode::MDNode(llvm::LLVMContext&, unsigned int, llvm::Metadata::StorageType, llvm::ArrayRef<llvm::Metadata*>, llvm::ArrayRef<llvm::Metadata*>) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/IR/Metadata.cpp:525:5
    #4 0x1d81000 in DINode /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/include/llvm/IR/DebugInfoMetadata.h:125:9
    #5 0x1d81000 in DIScope /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/include/llvm/IR/DebugInfoMetadata.h:472:9
    #6 0x1d81000 in DIType /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/include/llvm/IR/DebugInfoMetadata.h:665:9
    #7 0x1d81000 in DIDerivedType /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/include/llvm/IR/DebugInfoMetadata.h:921:9
    #8 0x1d81000 in llvm::DIDerivedType::getImpl(llvm::LLVMContext&, unsigned int, llvm::MDString*, llvm::Metadata*, unsigned int, llvm::Metadata*, llvm::Metadata*, unsigned long, unsigned int, unsigned long, llvm::Optional<unsigned int>, llvm::DINode::DIFlags, llvm::Metadata*, llvm::Metadata::StorageType, bool) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/IR/DebugInfoMetadata.cpp:577:3
    #9 0x1d42be4 in getImpl /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/include/llvm/IR/DebugInfoMetadata.h:932:12
    #10 0x1d42be4 in get /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/include/llvm/IR/DebugInfoMetadata.h:963:3
    #11 0x1d42be4 in llvm::DIBuilder::createSetType(llvm::DIScope*, llvm::StringRef, llvm::DIFile*, unsigned int, unsigned long, unsigned int, llvm::DIType*) /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/lib/IR/DIBuilder.cpp:533:7
    #12 0xbd0fdc in (anonymous namespace)::DIBuilder_CreateSetType_Test::TestBody() /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/unittests/IR/DebugInfoTest.cpp:248:32
    #13 0x35266b3 in HandleExceptionsInMethodIfSupported<testing::Test, void> /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc
    #14 0x35266b3 in testing::Test::Run() /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc:2474:5
    #15 0x3528ffe in testing::TestInfo::Run() /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc:2656:11
    #16 0x352a570 in testing::TestCase::Run() /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc:2774:28
    #17 0x35431f5 in testing::internal::UnitTestImpl::RunAllTests() /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc:4649:43
    #18 0x3541a92 in HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool> /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc
    #19 0x3541a92 in testing::UnitTest::Run() /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/utils/unittest/googletest/src/gtest.cc:4257:10
    #20 0x350d45f in RUN_ALL_TESTS /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/utils/unittest/googletest/include/gtest/gtest.h:2233:46
    #21 0x350d45f in main /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/utils/unittest/UnitTestMain/TestMain.cpp:50:10
    #22 0x7f7ced3dc09a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
    #23 0x922db9 in _start (/b/sanitizer-x86_64-linux-fast/build/llvm_build_msan/unittests/IR/IRTests+0x922db9)

SUMMARY: MemorySanitizer: use-of-uninitialized-value /b/sanitizer-x86_64-linux-fast/build/llvm-project/llvm/include/llvm/IR/Metadata.h:772:9 in track

[ostannard]

One of the tests was failing on the ARM/AArch64 bots because they don't build the X86 backend, I've moved that test to the X86-specific directory (rG6b3fb4714365).

[aprantl]

@vitalybuka looking...

@ostannard thanks!

[aprantl]

@MaskRay already fixed it! Thanks!

llvm/lib/IR/Verifier.cpp

AssertDI(N.getTag() == dwarf::DW_TAG_typedef ||
                N.getTag() == dwarf::DW_TAG_friend ||
                N.getTag() == dwarf::DW_TAG_set_type,
            "invalid tag", &N);
   if (N.getTag() == dwarf::DW_TAG_ptr_to_member_type) {
     AssertDI(isType(N.getRawExtraData()), "invalid pointer to member type", &N,
              N.getRawExtraData());
   }
 
+  if (N.getTag() == dwarf::DW_TAG_set_type) {
+    if (auto *T = N.getRawBaseType()) {
+      auto *Enum = dyn_cast_or_null<DICompositeType>(T);
+      auto *Basic = dyn_cast_or_null<DIBasicType>(T);
+      AssertDI((Enum && Enum->getTag() == dwarf::DW_TAG_enumeration_type) ||
+               (Basic && (Basic->getEncoding() == dwarf::DW_ATE_unsigned      ||
+                          Basic->getEncoding() == dwarf::DW_ATE_signed        ||
+                          Basic->getEncoding() == dwarf::DW_ATE_unsigned_char ||
+                          Basic->getEncoding() == dwarf::DW_ATE_signed_char)
+               ),
+               "invalid set base type", &N, T);
+    }
+  }
+
   AssertDI(isScope(N.getRawScope()), "invalid scope", &N, N.getRawScope());
   AssertDI(isType(N.getRawBaseType()), "invalid base type", &N,
            N.getRawBaseType());
 
   if (N.getDWARFAddressSpace()) {
     AssertDI(N.getTag() == dwarf::DW_TAG_pointer_type ||
                  N.getTag() == dwarf::DW_TAG_reference_type ||
                  N.getTag() == dwarf::DW_TAG_rvalue_reference_type,