-
Notifications
You must be signed in to change notification settings - Fork 13.4k
[clang][Modules] Fix the Size of RecordDecl
's BitCodeAbbrevOp
#133500
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
Conversation
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-modules Author: Qiongsi Wu (qiongsiwu) Changeshttps://github.com/llvm/llvm-project/pull/102040/files#diff-125f472e690aa3d973bc42aa3c5d580226c5c47661551aca2889f960681aa64dR2477 added 1 bit to This can lead to rare cases where a record may overflow if a Full diff: https://github.com/llvm/llvm-project/pull/133500.diff 1 Files Affected:
diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp
index a14b8cf201bba..f377c145a4204 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2579,7 +2579,7 @@ void ASTWriter::WriteDeclAbbrevs() {
// RecordDecl
Abv->Add(BitCodeAbbrevOp(
BitCodeAbbrevOp::Fixed,
- 13)); // Packed Record Decl Bits: FlexibleArrayMember,
+ 14)); // Packed Record Decl Bits: FlexibleArrayMember,
// AnonymousStructUnion, hasObjectMember, hasVolatileMember,
// isNonTrivialToPrimitiveDefaultInitialize,
// isNonTrivialToPrimitiveCopy, isNonTrivialToPrimitiveDestroy,
|
Note to reviewers: A test case is added by swiftlang#10371. This is difficult to test in the community llvm code base because the failure was discovered when building structs with More generally, should we hardcode the size in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for spotting & fixing this! Approving, but I'm also quite unfamiliar with these so please wait for another approval.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks.
…vm#133500) https://github.com/llvm/llvm-project/pull/102040/files#diff-125f472e690aa3d973bc42aa3c5d580226c5c47661551aca2889f960681aa64dR2477 added 1 bit to `RecordDecl`'s serialization format, but did not increment its abbreviation size. This can lead to rare cases where a record may overflow if the `RecordDecl`'s `getArgPassingRestrictions()` returns something bigger than 1 (see [here](https://github.com/llvm/llvm-project/blob/b3f01a6aa45b00240cec1c64286b85d7ba87e2af/clang/lib/Serialization/ASTWriterDecl.cpp#L688)). rdar://143763558
…vm#133500) https://github.com/llvm/llvm-project/pull/102040/files#diff-125f472e690aa3d973bc42aa3c5d580226c5c47661551aca2889f960681aa64dR2477 added 1 bit to `RecordDecl`'s serialization format, but did not increment its abbreviation size. This can lead to rare cases where a record may overflow if the `RecordDecl`'s `getArgPassingRestrictions()` returns something bigger than 1 (see [here](https://github.com/llvm/llvm-project/blob/b3f01a6aa45b00240cec1c64286b85d7ba87e2af/clang/lib/Serialization/ASTWriterDecl.cpp#L688)). rdar://143763558 (cherry picked from commit 4a73c99)
…10513 This is a test case for llvm#133500. This test case cannot be added to upstream llvm because upstream currently does not have the __ptrauth qualifier yet. llvm#100830 will add the qualifier, and we shall consider upstreaming this test case when llvm#100830 lands. rdar://143763558 (cherry picked from commit 3af0d75)
https://github.com/llvm/llvm-project/pull/102040/files#diff-125f472e690aa3d973bc42aa3c5d580226c5c47661551aca2889f960681aa64dR2477 added 1 bit to
RecordDecl
's serialization format, but did not increment its abbreviation size.This can lead to rare cases where a record may overflow if the
RecordDecl
'sgetArgPassingRestrictions()
returns something bigger than 1 (see here).rdar://143763558