LLVM Bugzilla is read-only and represents the historical archive of all LLVM issues filled before November 26, 2021. Use github to submit LLVM bugs

Bug 11652 - assertion failures when Type.cpp is compiled with -Os
Summary: assertion failures when Type.cpp is compiled with -Os
Status: RESOLVED FIXED
Alias: None
Product: new-bugs
Classification: Unclassified
Component: new bugs (show other bugs)
Version: trunk
Hardware: PC Linux
: P enhancement
Assignee: Unassigned LLVM Bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-12-26 17:30 PST by nobled
Modified: 2012-01-05 05:33 PST (History)
5 users (show)

See Also:
Fixed By Commit(s):

Attachments
PATCH (738 bytes, patch)
2011-12-31 07:26 PST, Stepan Dyatkovskiy 		Details
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description nobled 2011-12-26 17:30:47 PST 
This is probably a GCC bug, but does anyone have advice on how to reduce it further?

GCC version:
g++ (Ubuntu/Linaro 4.5.2-8ubuntu4) 4.5.2

With r147274 and configuring with these options:

cmake ../llvm -DCMAKE_BUILD_TYPE=Debug -DLLVM_BUILD_CLANG=OFF -DBUILD_SHARED_LIBS=ON -DCMAKE_CXX_FLAGS=-Os

It causes assertion failures in a ton of unit tests:

.../include/llvm/Type.h:296: llvm::Type* llvm::Type::getContainedType(unsigned int) const: Assertion `i < NumContainedTys && "Index out of range!"' failed.
Stack dump:
0.	Running pass 'X86 DAG->DAG Instruction Selection' on function '@foo1'

.../lib/Bitcode/Writer/ValueEnumerator.h:94: unsigned int llvm::ValueEnumerator::getTypeID(llvm::Type*) const: Assertion `I != TypeMap.end() && "Type not in ValueEnumerator!"' failed.
Stack dump:
0.	Program arguments: bin/./opt -strip-debug 
1.	Running pass 'Bitcode Writer' on module '<stdin>'.

.../lib/Bitcode/Reader/BitcodeReader.cpp:740: bool llvm::BitcodeReader::ParseTypeTableBody(): Assertion `TypeList[NumRecords] == 0 && "Already read type?"' failed.

  Expected Passes    : 4167
  Expected Failures  : 50
  Unsupported Tests  : 13
  Unexpected Failures: 1437


Taking off that -DCMAKE_CXX_FLAGS=-Os makes the assertions go away. And copying Type.cpp.o from a build *without* -Os into a build that was configured *with* it also fixes them (where -b2 was a good build, and the current directory is a bad one):
cp -ai ../llvm-b2/lib/VMCore/CMakeFiles/LLVMCore.dir/Type.cpp.o lib/VMCore/CMakeFiles/LLVMCore.dir/
Comment 1 Eli Friedman 2011-12-26 17:56:47 PST 
You can try splitting Type.cpp into two files, put half the definitions into one file, see which file contains the miscomputed function, etc.
Comment 2 nobled 2011-12-27 06:19:40 PST 
An 'interesting' result on that front: Depending on how I split the functions between the two new files, sometimes the assertions disappear entirely. Still figuring out which *combination* of functions being in the same translation unit causes it...
Comment 3 nobled 2011-12-30 14:41:20 PST 
I split Type.cpp into two files. PointerType's constructor seems to be the culprit function, because moving it between the two files changes which object file needs to be replaced to fix the assertions. However, if it's in a file by itself, the assertions don't occur.

Right now, the minimal combination I found that causes failure is this, with just one other function definition in the file:

#include "LLVMContextImpl.h"
using namespace llvm;

void StructType::setBody(ArrayRef<Type*> Elements, bool isPacked) {
  assert(isOpaque() && "Struct body already set!");
  
  setSubclassData(getSubclassData() | SCDB_HasBody);
  if (isPacked)
    setSubclassData(getSubclassData() | SCDB_Packed);
  
  Type **Elts = getContext().pImpl->
    TypeAllocator.Allocate<Type*>(Elements.size());
  memcpy(Elts, Elements.data(), sizeof(Elements[0])*Elements.size());
  
  ContainedTys = Elts;
  NumContainedTys = Elements.size();
}

PointerType::PointerType(Type *E, unsigned AddrSpace)
  : SequentialType(PointerTyID, E) {
  setSubclassData(AddrSpace);
}
Comment 4 Eli Friedman 2011-12-30 16:58:16 PST 
Okay... at that point, all you can really do is try to separate out an executable testcase that crashes from the rest of LLVM, and file a bug in the gcc or Ubuntu bug tracker...
Comment 5 nobled 2011-12-30 19:41:55 PST 
Huh, I guess it's bitfield shenanigans going by this patch?

http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20111226/134181.html
Comment 6 Eli Friedman 2011-12-30 22:20:46 PST 
Yes, likely the same issue... did anyone actually file it against gcc, though?
Comment 7 Stepan Dyatkovskiy 2011-12-31 07:26:50 PST 
Created attachment 7823 [details]
PATCH

Guys, I fixed that. I posted it to llvm-commits. But it seems that it was deeply dived in posts ocean. So I repost it here:

The problem is in Type.h. The fields in Type class are declared in next
order:
   TypeID   ID : 8;
   unsigned SubclassData : 24;
   unsigned NumContainedTys;

Attempt to set new SubclassData value rewrites lowest byte in
NumContainedTys when -Os is set. GCC bug? Anyway setting SubclassData
with two workaround strings fixes the problem:

  void setSubclassData(unsigned val) {
     unsigned tmp = NumContainedTys;  // Workaround for GCC -Os
     SubclassData = val;
     NumContainedTys = tmp;  // Workaround for GCC -Os
     // Ensure we don't have any accidental truncation.
     assert(SubclassData == val && "Subclass data too large for field");
   }

Probably there is another ways to protect NumContainedTys from overwritting?

Please find the patch in attachment for review.

-Stepan.
Comment 8 nobled 2011-12-31 07:46:43 PST 
(In reply to comment #6)
> Yes, likely the same issue... did anyone actually file it against gcc, though?
I was goint to try and make a standalone single-file executable, but it's ridiculously complicated. So yeah, filed a bug with Ubuntu with the details for now:

https://bugs.launchpad.net/bugs/910363
Comment 9 Stepan Dyatkovskiy 2012-01-04 01:45:41 PST 
Fixed in r147470
Comment 10 nobled 2012-01-05 04:53:34 PST 
For the record, I just filed a bug with GCC:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51759