diff --git a/clang/lib/Analysis/UnsafeBufferUsage.cpp b/clang/lib/Analysis/UnsafeBufferUsage.cpp --- a/clang/lib/Analysis/UnsafeBufferUsage.cpp +++ b/clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -1385,8 +1385,18 @@ TypeLoc PteTyLoc = TyLoc.getUnqualifiedLoc().getNextTypeLoc(); SourceLocation VDNameStartLoc = VD->getLocation(); - if (!SM.isBeforeInTranslationUnit(PteTyLoc.getSourceRange().getEnd(), - VDNameStartLoc)) { + if (!(VDNameStartLoc.isValid() && PteTyLoc.getSourceRange().isValid())) { + // We are expecting these locations to be valid. But in some cases, they are + // not all valid. It is a Clang bug to me and we are not responsible for + // fixing it. So we will just give up for now when it happens. + return std::nullopt; + } + + // Note that TypeLoc.getEndLoc() returns the begin location of the last token: + SourceLocation PteEndOfTokenLoc = + Lexer::getLocForEndOfToken(PteTyLoc.getEndLoc(), 0, SM, LangOpts); + + if (!SM.isBeforeInTranslationUnit(PteEndOfTokenLoc, VDNameStartLoc)) { // We only deal with the cases where the source text of the pointee type // appears on the left-hand side of the variable identifier completely, // including the following forms: @@ -1401,13 +1411,8 @@ // `PteTy` via source ranges. *QualifiersToAppend = PteTy.getQualifiers(); } - - // Note that TypeLoc.getEndLoc() returns the begin location of the last token: - SourceRange CSR{ - PteTyLoc.getBeginLoc(), - Lexer::getLocForEndOfToken(PteTyLoc.getEndLoc(), 0, SM, LangOpts)}; - - return getRangeText(CSR, SM, LangOpts)->str(); + return getRangeText({PteTyLoc.getBeginLoc(), PteEndOfTokenLoc}, SM, LangOpts) + ->str(); } // Returns the text of the name (with qualifiers) of a `FunctionDecl`. diff --git a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-unsupported.cpp b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-unsupported.cpp --- a/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-unsupported.cpp +++ b/clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-parm-unsupported.cpp @@ -33,6 +33,26 @@ } } +// The analysis requires accurate source location informations from +// `TypeLoc`s of types of variable (parameter) declarations in order +// to generate fix-its for them. But those information is not always +// available (probably due to some bugs in clang but it is irrelevant +// to the safe-buffer project). The following is an example. When +// `_Atomic` is used, we cannot get valid source locations of the +// pointee type of `unsigned *`. The analysis gives up in such a +// case. +// CHECK-NOT: fix-it: +void typeLocSourceLocationInvalid(_Atomic unsigned *map) { // expected-warning{{'map' is an unsafe pointer used for buffer access}} + map[5] = 5; // expected-note{{used in buffer access here}} +} + +// CHECK: fix-it:"{{.*}}":{[[@LINE+1]]:33-[[@LINE+1]]:46}:"std::span map" +void typeLocSourceLocationValid(unsigned *map) { // expected-warning{{'map' is an unsafe pointer used for buffer access}} \ + expected-note{{change type of 'map' to 'std::span' to preserve bounds information}} + map[5] = 5; // expected-note{{used in buffer access here}} +} +// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:2-[[@LINE-1]]:2}:"\n{{\[}}{{\[}}clang::unsafe_buffer_usage{{\]}}{{\]}} void typeLocSourceLocationValid(unsigned *map) {return typeLocSourceLocationValid(std::span(map, <# size #>));}\n" + // We do not fix parameters participating unsafe operations for the // following functions/methods or function-like expressions: @@ -128,4 +148,3 @@ int tmp; tmp = x[5]; // expected-note{{used in buffer access here}} } -