Skip to content

Commit 9d1dd9a

Browse files
authored
[Sema] Fix false positive warnings for misaligned member access (#150025)
These warnings are reported on a per expression basis, however some potential misaligned accesses are discarded before that happens. The problem is when a new expression starts while processing another expression. The new expression will end first and emit all potential misaligned accesses collected up to that point. That includes candidates that were found in the parent expression, even though they might have gotten discarded later. Fixed by checking if the candidate is located withing the currently processed expression. Fixes #144729
1 parent 2211394 commit 9d1dd9a

File tree

5 files changed

+30
-13
lines changed

5 files changed

+30
-13
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,10 @@ Improvements to Clang's diagnostics
168168
"signedness of format specifier 'u' is incompatible with 'c' [-Wformat-signedness]"
169169
and the API-visible diagnostic id will be appropriate.
170170

171+
- Fixed false positives in ``-Waddress-of-packed-member`` diagnostics when
172+
potential misaligned members get processed before they can get discarded.
173+
(#GH144729)
174+
171175
Improvements to Clang's time-trace
172176
----------------------------------
173177

clang/include/clang/Sema/Sema.h

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2659,9 +2659,9 @@ class Sema final : public SemaBase {
26592659
/// identifies the magic value.
26602660
typedef std::pair<const IdentifierInfo *, uint64_t> TypeTagMagicValue;
26612661

2662-
/// Diagnoses the current set of gathered accesses. This typically
2663-
/// happens at full expression level. The set is cleared after emitting the
2664-
/// diagnostics.
2662+
/// Diagnoses the current set of gathered accesses. This happens at the end of
2663+
/// each expression evaluation context. Diagnostics are emitted only for
2664+
/// accesses gathered in the current evaluation context.
26652665
void DiagnoseMisalignedMembers();
26662666

26672667
/// This function checks if the expression is in the sef of potentially
@@ -3117,9 +3117,6 @@ class Sema final : public SemaBase {
31173117

31183118
bool operator==(const MisalignedMember &m) { return this->E == m.E; }
31193119
};
3120-
/// Small set of gathered accesses to potentially misaligned members
3121-
/// due to the packed attribute.
3122-
SmallVector<MisalignedMember, 4> MisalignedMembers;
31233120

31243121
/// Adds an expression to the set of gathered misaligned members.
31253122
void AddPotentialMisalignedMembers(Expr *E, RecordDecl *RD, ValueDecl *MD,
@@ -6765,6 +6762,10 @@ class Sema final : public SemaBase {
67656762
/// InLifetimeExtendingContext is true.
67666763
SmallVector<MaterializeTemporaryExpr *, 8> ForRangeLifetimeExtendTemps;
67676764

6765+
/// Small set of gathered accesses to potentially misaligned members
6766+
/// due to the packed attribute.
6767+
SmallVector<MisalignedMember, 4> MisalignedMembers;
6768+
67686769
/// \brief Describes whether we are in an expression constext which we have
67696770
/// to handle differently.
67706771
enum ExpressionKind {

clang/lib/Sema/SemaChecking.cpp

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14125,7 +14125,6 @@ void Sema::CheckCompletedExpr(Expr *E, SourceLocation CheckLoc,
1412514125
CheckUnsequencedOperations(E);
1412614126
if (!IsConstexpr && !E->isValueDependent())
1412714127
CheckForIntOverflow(E);
14128-
DiagnoseMisalignedMembers();
1412914128
}
1413014129

1413114130
void Sema::CheckBitFieldInitialization(SourceLocation InitLoc,
@@ -15570,11 +15569,12 @@ void Sema::CheckArgumentWithTypeTag(const ArgumentWithTypeTagAttr *Attr,
1557015569

1557115570
void Sema::AddPotentialMisalignedMembers(Expr *E, RecordDecl *RD, ValueDecl *MD,
1557215571
CharUnits Alignment) {
15573-
MisalignedMembers.emplace_back(E, RD, MD, Alignment);
15572+
currentEvaluationContext().MisalignedMembers.emplace_back(E, RD, MD,
15573+
Alignment);
1557415574
}
1557515575

1557615576
void Sema::DiagnoseMisalignedMembers() {
15577-
for (MisalignedMember &m : MisalignedMembers) {
15577+
for (MisalignedMember &m : currentEvaluationContext().MisalignedMembers) {
1557815578
const NamedDecl *ND = m.RD;
1557915579
if (ND->getName().empty()) {
1558015580
if (const TypedefNameDecl *TD = m.RD->getTypedefNameForAnonDecl())
@@ -15583,7 +15583,7 @@ void Sema::DiagnoseMisalignedMembers() {
1558315583
Diag(m.E->getBeginLoc(), diag::warn_taking_address_of_packed_member)
1558415584
<< m.MD << ND << m.E->getSourceRange();
1558515585
}
15586-
MisalignedMembers.clear();
15586+
currentEvaluationContext().MisalignedMembers.clear();
1558715587
}
1558815588

1558915589
void Sema::DiscardMisalignedMemberAddress(const Type *T, Expr *E) {
@@ -15594,13 +15594,15 @@ void Sema::DiscardMisalignedMemberAddress(const Type *T, Expr *E) {
1559415594
cast<UnaryOperator>(E)->getOpcode() == UO_AddrOf) {
1559515595
auto *Op = cast<UnaryOperator>(E)->getSubExpr()->IgnoreParens();
1559615596
if (isa<MemberExpr>(Op)) {
15597-
auto *MA = llvm::find(MisalignedMembers, MisalignedMember(Op));
15598-
if (MA != MisalignedMembers.end() &&
15597+
auto &MisalignedMembersForExpr =
15598+
currentEvaluationContext().MisalignedMembers;
15599+
auto *MA = llvm::find(MisalignedMembersForExpr, MisalignedMember(Op));
15600+
if (MA != MisalignedMembersForExpr.end() &&
1559915601
(T->isDependentType() || T->isIntegerType() ||
1560015602
(T->isPointerType() && (T->getPointeeType()->isIncompleteType() ||
1560115603
Context.getTypeAlignInChars(
1560215604
T->getPointeeType()) <= MA->Alignment))))
15603-
MisalignedMembers.erase(MA);
15605+
MisalignedMembersForExpr.erase(MA);
1560415606
}
1560515607
}
1560615608
}

clang/lib/Sema/SemaExpr.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18121,6 +18121,8 @@ void Sema::PopExpressionEvaluationContext() {
1812118121
MaybeODRUseExprs.insert_range(Rec.SavedMaybeODRUseExprs);
1812218122
}
1812318123

18124+
DiagnoseMisalignedMembers();
18125+
1812418126
// Pop the current expression evaluation context off the stack.
1812518127
ExprEvalContexts.pop_back();
1812618128
}

clang/test/Sema/address-packed.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,3 +338,11 @@ struct Invalid0 {
338338
void *g14(struct Invalid0 *ivl) {
339339
return &(ivl->x);
340340
}
341+
342+
void to_void_with_expr(void *ptr, int expr);
343+
344+
void g15(void) {
345+
struct Arguable arguable;
346+
to_void_with_expr(&arguable.x, 3); // no-warning
347+
to_void_with_expr(&arguable.x, ({3;})); // no-warning
348+
}

0 commit comments

Comments
 (0)