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 11751 - Unmatched BaseTy in Assertion Test in ComputeRecordLayout function
Summary: Unmatched BaseTy in Assertion Test in ComputeRecordLayout function
Status: RESOLVED FIXED
Alias: None
Product: clang
Classification: Unclassified
Component: libclang (show other bugs)
Version: trunk
Hardware: Other Linux
: P enhancement
Assignee: Unassigned Clang Bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-01-12 15:13 PST by yinma
Modified: 2012-01-12 21:59 PST (History)
1 user (show)

See Also:
Fixed By Commit(s):


Attachments
File to produce assertion, --enable-assertion=YES is required (58 bytes, application/octet-stream)
2012-01-12 15:13 PST, yinma
Details
The patch I proprosed (633 bytes, patch)
2012-01-12 15:14 PST, yinma
Details
Validation File 1 (40.94 KB, text/plain)
2012-01-12 15:15 PST, yinma
Details
Validation File 2 (1.95 KB, application/octet-stream)
2012-01-12 15:15 PST, yinma
Details
This is the real t.c to produce the assertion. --enable-assertion is required (157 bytes, application/octet-stream)
2012-01-12 15:57 PST, yinma
Details

Note You need to log in before you can comment on or make changes to this bug.
Description yinma 2012-01-12 15:13:29 PST
Created attachment 7867 [details]
File to produce assertion,  --enable-assertion=YES is required

This assertion in CGRecordLayout *CodeGenTypes::ComputeRecordLayout(const RecordDecl *D, llvm::StructType *Ty) bothers Peren test. It makes it failed. This assertion tried to compute the size of BaseTy of a layout's non virtual size. However, the real code has modified the BaseTy before assertion. So I added a recompution of the real BaseTy in assert test segment 

...
  // If we're in C++, compute the base subobject type.
  llvm::StructType *BaseTy = 0;
  if (isa<CXXRecordDecl>(D)) {
    BaseTy = Builder.BaseSubobjectType;
    if (!BaseTy) BaseTy = Ty;   <--- is not BaseTy anymore if BaseTy is NULL
  }
...
#ifndef NDEBUG
  // Verify that the computed LLVM struct size matches the AST layout size.
  const ASTRecordLayout &Layout = getContext().getASTRecordLayout(D);
...

  if (BaseTy) {  <-- this requires the true BaseTy
    CharUnits NonVirtualSize  = Layout.getNonVirtualSize();
    CharUnits NonVirtualAlign = Layout.getNonVirtualAlign();
    CharUnits AlignedNonVirtualTypeSize = 
      NonVirtualSize.RoundUpToAlignment(NonVirtualAlign);

    uint64_t AlignedNonVirtualTypeSizeInBits = 
      getContext().toBits(AlignedNonVirtualTypeSize);

    assert(AlignedNonVirtualTypeSizeInBits == 
           getTargetData().getTypeAllocSizeInBits(BaseTy) &&
           "Type size mismatch!");
  }
Comment 1 yinma 2012-01-12 15:14:01 PST
I have a potential fix already, please see clang.diff attached
Comment 2 yinma 2012-01-12 15:14:37 PST
Created attachment 7868 [details]
The patch I proprosed
Comment 3 yinma 2012-01-12 15:15:11 PST
Created attachment 7869 [details]
Validation File 1
Comment 4 yinma 2012-01-12 15:15:32 PST
Created attachment 7870 [details]
Validation File 2
Comment 5 yinma 2012-01-12 15:16:54 PST
I recompute the BaseTy in the assertion segment to deal with this problem. I have run both cross and simple cross test validation with this change. No new failure is introduced. Please give a review, Thanks.
Comment 6 yinma 2012-01-12 15:57:55 PST
Created attachment 7872 [details]
This is the real t.c to produce the assertion. --enable-assertion is required
Comment 7 Eli Friedman 2012-01-12 21:59:38 PST
r148093.