Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NFC] Adapt unit tests to new num_extra_inhabitants field #7662

Merged
merged 1 commit into from
Oct 23, 2023

Conversation

augusto2112
Copy link

No description provided.

@augusto2112 augusto2112 merged commit c437b65 into swiftlang:stable/20230725 Oct 23, 2023
augusto2112 referenced this pull request Oct 23, 2023
Swift types have the concept of "extra inhabitants". An extra inhabitant
is a bit pattern that does not represent a valid value for objects of a
given type. Add a new Apple DWARF extension,
APPLE_num_extra_inhabitants, to encode this information in DWARF.
augusto2112 pushed a commit that referenced this pull request Oct 26, 2023
After 89d458b and #7662, the code of
MetadataLoader.cpp allows the new NumExtraInhabitants field, but it
always supposes it is available. Some tests do not seem to like that.
When compiling with asserts, binaries like `opt` will crash with an
assertion accessing `SmallVector` indices out of bounts, while in
no-asserts it just keeps going, probably using garbage values.

Change two places where the `Record` was accessed with `operator[]`
without first checking for the `Record` size to check for the size. Use
a default of `0` in case the record is not found.

The test that failed before these changes are:

```
  LLVM :: Bitcode/DIExpression-4.0.ll
  LLVM :: Bitcode/DIExpression-aggresult.ll
  LLVM :: Bitcode/DIExpression-deref.ll
  LLVM :: Bitcode/DIExpression-minus-upgrade.ll
  LLVM :: Bitcode/DIGlobalVariableExpression.ll
  LLVM :: Bitcode/DIGlobalVariableExpression2.ll
  LLVM :: Bitcode/DIModule-fortran-module.ll
  LLVM :: Bitcode/DINamespace.ll
  LLVM :: Bitcode/DISubprogram-v4.ll
  LLVM :: Bitcode/DISubprogram-v5.ll
  LLVM :: Bitcode/DITemplateParameter-5.0.ll
  LLVM :: Bitcode/diglobalvariable-3.8.ll
  LLVM :: Bitcode/dilocalvariable-3.9.ll
  LLVM :: Bitcode/disubrange-v0.ll
  LLVM :: Bitcode/dityperefs-3.8.ll
  LLVM :: Bitcode/invalid.test
  LLVM :: Bitcode/upgrade-cu-locals.ll
  LLVM :: Bitcode/upgrade-dbg-addr.ll
  LLVM :: Bitcode/upgrade-dbg-value.ll
  LLVM :: Bitcode/upgrade-pointer-address-space.ll
  LLVM :: ThinLTO/X86/drop-debug-info.ll
```

The failure looked like the following:

```
  Assertion failed: (idx < size()), function operator[], file SmallVector.h, line 294.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.	Program arguments: /Users/danielrodriguez/code/swift-source/build/my_macos/llvm-macosx-x86_64/bin/llvm-dis -o - /Users/danielrodriguez/code/swift-source/llvm-project/llvm/test/Bitcode/DIExpression-minus-upgrade.ll.bc
 #0 0x00000001003507b7 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/Users/danielrodriguez/code/swift-source/build/my_macos/llvm-macosx-x86_64/bin/llvm-dis+0x1001ca7b7)
 #1 0x000000010034ed55 llvm::sys::RunSignalHandlers() (/Users/danielrodriguez/code/swift-source/build/my_macos/llvm-macosx-x86_64/bin/llvm-dis+0x1001c8d55)
 #2 0x0000000100351040 SignalHandler(int) (/Users/danielrodriguez/code/swift-source/build/my_macos/llvm-macosx-x86_64/bin/llvm-dis+0x1001cb040)
 #3 0x00007ff805ec05ed (/usr/lib/system/libsystem_platform.dylib+0x7ff8004245ed)
 #4 0x0000000000000000
 #5 0x00007ff805db9b45 (/usr/lib/system/libsystem_c.dylib+0x7ff80031db45)
 #6 0x00007ff805db8e5e (/usr/lib/system/libsystem_c.dylib+0x7ff80031ce5e)
 #7 0x0000000100385573 llvm::MetadataLoader::MetadataLoaderImpl::parseOneMetadata(llvm::SmallVectorImpl<unsigned long long>&, unsigned int, (anonymous namespace)::(anonymous namespace)::PlaceholderQueue&, llvm::StringRef, unsigned int&) (.cold.50) (/Users/danielrodriguez/code/swift-source/build/my_macos/llvm-macosx-x86_64/bin/llvm-dis+0x1001ff573)
 #8 0x00000001001d13a3 llvm::MetadataLoader::MetadataLoaderImpl::parseOneMetadata(llvm::SmallVectorImpl<unsigned long long>&, unsigned int, (anonymous namespace)::(anonymous namespace)::PlaceholderQueue&, llvm::StringRef, unsigned int&) (/Users/danielrodriguez/code/swift-source/build/my_macos/llvm-macosx-x86_64/bin/llvm-dis+0x10004b3a3)
 #9 0x00000001001c9496 llvm::MetadataLoader::MetadataLoaderImpl::parseMetadata(bool) (/Users/danielrodriguez/code/swift-source/build/my_macos/llvm-macosx-x86_64/bin/llvm-dis+0x100043496)
 #10 0x00000001001d2f71 llvm::MetadataLoader::parseMetadata(bool) (/Users/danielrodriguez/code/swift-source/build/my_macos/llvm-macosx-x86_64/bin/llvm-dis+0x10004cf71)
 #11 0x00000001001b33b7 (anonymous namespace)::BitcodeReader::parseModule(unsigned long long, bool, llvm::ParserCallbacks) (/Users/danielrodriguez/code/swift-source/build/my_macos/llvm-macosx-x86_64/bin/llvm-dis+0x10002d3b7)
 #12 0x000000010018f2f5 llvm::BitcodeModule::getModuleImpl(llvm::LLVMContext&, bool, bool, bool, llvm::ParserCallbacks) (/Users/danielrodriguez/code/swift-source/build/my_macos/llvm-macosx-x86_64/bin/llvm-dis+0x1000092f5)
 #13 0x000000010019008a llvm::BitcodeModule::getLazyModule(llvm::LLVMContext&, bool, bool, llvm::ParserCallbacks) (/Users/danielrodriguez/code/swift-source/build/my_macos/llvm-macosx-x86_64/bin/llvm-dis+0x10000a08a)
 #14 0x0000000100189593 main (/Users/danielrodriguez/code/swift-source/build/my_macos/llvm-macosx-x86_64/bin/llvm-dis+0x100003593)
 #15 0x0000000200a0941f
```

With these changes, those tests all pass and the crash does not
reproduce.
augusto2112 pushed a commit that referenced this pull request Feb 20, 2024
After 89d458b and #7662, the code of
MetadataLoader.cpp allows the new NumExtraInhabitants field, but it
always supposes it is available. Some tests do not seem to like that.
When compiling with asserts, binaries like `opt` will crash with an
assertion accessing `SmallVector` indices out of bounts, while in
no-asserts it just keeps going, probably using garbage values.

Change two places where the `Record` was accessed with `operator[]`
without first checking for the `Record` size to check for the size. Use
a default of `0` in case the record is not found.

The test that failed before these changes are:

```
  LLVM :: Bitcode/DIExpression-4.0.ll
  LLVM :: Bitcode/DIExpression-aggresult.ll
  LLVM :: Bitcode/DIExpression-deref.ll
  LLVM :: Bitcode/DIExpression-minus-upgrade.ll
  LLVM :: Bitcode/DIGlobalVariableExpression.ll
  LLVM :: Bitcode/DIGlobalVariableExpression2.ll
  LLVM :: Bitcode/DIModule-fortran-module.ll
  LLVM :: Bitcode/DINamespace.ll
  LLVM :: Bitcode/DISubprogram-v4.ll
  LLVM :: Bitcode/DISubprogram-v5.ll
  LLVM :: Bitcode/DITemplateParameter-5.0.ll
  LLVM :: Bitcode/diglobalvariable-3.8.ll
  LLVM :: Bitcode/dilocalvariable-3.9.ll
  LLVM :: Bitcode/disubrange-v0.ll
  LLVM :: Bitcode/dityperefs-3.8.ll
  LLVM :: Bitcode/invalid.test
  LLVM :: Bitcode/upgrade-cu-locals.ll
  LLVM :: Bitcode/upgrade-dbg-addr.ll
  LLVM :: Bitcode/upgrade-dbg-value.ll
  LLVM :: Bitcode/upgrade-pointer-address-space.ll
  LLVM :: ThinLTO/X86/drop-debug-info.ll
```

The failure looked like the following:

```
  Assertion failed: (idx < size()), function operator[], file SmallVector.h, line 294.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Stack dump:
0.	Program arguments: /Users/danielrodriguez/code/swift-source/build/my_macos/llvm-macosx-x86_64/bin/llvm-dis -o - /Users/danielrodriguez/code/swift-source/llvm-project/llvm/test/Bitcode/DIExpression-minus-upgrade.ll.bc
 #0 0x00000001003507b7 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/Users/danielrodriguez/code/swift-source/build/my_macos/llvm-macosx-x86_64/bin/llvm-dis+0x1001ca7b7)
 #1 0x000000010034ed55 llvm::sys::RunSignalHandlers() (/Users/danielrodriguez/code/swift-source/build/my_macos/llvm-macosx-x86_64/bin/llvm-dis+0x1001c8d55)
 #2 0x0000000100351040 SignalHandler(int) (/Users/danielrodriguez/code/swift-source/build/my_macos/llvm-macosx-x86_64/bin/llvm-dis+0x1001cb040)
 #3 0x00007ff805ec05ed (/usr/lib/system/libsystem_platform.dylib+0x7ff8004245ed)
 #4 0x0000000000000000
 #5 0x00007ff805db9b45 (/usr/lib/system/libsystem_c.dylib+0x7ff80031db45)
 #6 0x00007ff805db8e5e (/usr/lib/system/libsystem_c.dylib+0x7ff80031ce5e)
 #7 0x0000000100385573 llvm::MetadataLoader::MetadataLoaderImpl::parseOneMetadata(llvm::SmallVectorImpl<unsigned long long>&, unsigned int, (anonymous namespace)::(anonymous namespace)::PlaceholderQueue&, llvm::StringRef, unsigned int&) (.cold.50) (/Users/danielrodriguez/code/swift-source/build/my_macos/llvm-macosx-x86_64/bin/llvm-dis+0x1001ff573)
 #8 0x00000001001d13a3 llvm::MetadataLoader::MetadataLoaderImpl::parseOneMetadata(llvm::SmallVectorImpl<unsigned long long>&, unsigned int, (anonymous namespace)::(anonymous namespace)::PlaceholderQueue&, llvm::StringRef, unsigned int&) (/Users/danielrodriguez/code/swift-source/build/my_macos/llvm-macosx-x86_64/bin/llvm-dis+0x10004b3a3)
 #9 0x00000001001c9496 llvm::MetadataLoader::MetadataLoaderImpl::parseMetadata(bool) (/Users/danielrodriguez/code/swift-source/build/my_macos/llvm-macosx-x86_64/bin/llvm-dis+0x100043496)
 #10 0x00000001001d2f71 llvm::MetadataLoader::parseMetadata(bool) (/Users/danielrodriguez/code/swift-source/build/my_macos/llvm-macosx-x86_64/bin/llvm-dis+0x10004cf71)
 #11 0x00000001001b33b7 (anonymous namespace)::BitcodeReader::parseModule(unsigned long long, bool, llvm::ParserCallbacks) (/Users/danielrodriguez/code/swift-source/build/my_macos/llvm-macosx-x86_64/bin/llvm-dis+0x10002d3b7)
 #12 0x000000010018f2f5 llvm::BitcodeModule::getModuleImpl(llvm::LLVMContext&, bool, bool, bool, llvm::ParserCallbacks) (/Users/danielrodriguez/code/swift-source/build/my_macos/llvm-macosx-x86_64/bin/llvm-dis+0x1000092f5)
 #13 0x000000010019008a llvm::BitcodeModule::getLazyModule(llvm::LLVMContext&, bool, bool, llvm::ParserCallbacks) (/Users/danielrodriguez/code/swift-source/build/my_macos/llvm-macosx-x86_64/bin/llvm-dis+0x10000a08a)
 #14 0x0000000100189593 main (/Users/danielrodriguez/code/swift-source/build/my_macos/llvm-macosx-x86_64/bin/llvm-dis+0x100003593)
 #15 0x0000000200a0941f
```

With these changes, those tests all pass and the crash does not
reproduce.

(cherry picked from commit dc75586)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant