[clang] [clang/AST] Make it possible to use SwiftAttr in type context (PR #108631)
Xazax-hun wrote: I think the interesting template cases are like things like: ``` template struct S { T foo(); }; S a; S b; void f() { a.foo(); b.foo(); } ``` The question is, whether we actually have the type attributes where we expect to have them, or do we lose them? Hopefully @erichkeane can correct me if I'm wrong. That being said, one question is whether we actually need type attributes to be propagated in this case for Swift's interop. For the release notes, you'd want to mention that `swift_attr` can now be applied to types (maybe a small motivation why) in `clang/docs/ReleaseNotes.rst`. It has a section for attribute changes. https://github.com/llvm/llvm-project/pull/108631 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang/AST] Make it possible to use SwiftAttr in type context (PR #108631)
https://github.com/Xazax-hun edited https://github.com/llvm/llvm-project/pull/108631 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang/AST] Make it possible to use SwiftAttr in type context (PR #108631)
@@ -7147,6 +7147,60 @@ static bool HandleWebAssemblyFuncrefAttr(TypeProcessingState &State, return false; } +static void HandleSwiftAttr(TypeProcessingState &State, TypeAttrLocation TAL, +QualType &QT, ParsedAttr &PAttr) { + if (TAL == TAL_DeclName) +return; + + Sema &S = State.getSema(); + auto &D = State.getDeclarator(); + + // If the attribute appears in declaration specifiers + // it should be handled as a declaration attribute, + // unless it's associated with a type or a function + // prototype (i.e. appears on a parameter or result type). + if (State.isProcessingDeclSpec()) { +if (!(D.isPrototypeContext() || + D.getContext() == DeclaratorContext::TypeName)) + return; + +if (auto *chunk = D.getInnermostNonParenChunk()) { + moveAttrFromListToList(PAttr, State.getCurrentAttributes(), + const_cast(chunk)->getAttrs()); + return; +} + } + + StringRef Str; + if (!S.checkStringLiteralArgumentAttr(PAttr, 0, Str)) { +PAttr.setInvalid(); +return; + } + + // If the attribute as attached to a paren move it closer to + // the declarator. This can happen in block declarations when + // an attribute is placed before `^` i.e. `(__attribute__((...)) ^)`. Xazax-hun wrote: One way to test it is to print the AST and use FileCheck to match on the AST dump. https://github.com/llvm/llvm-project/pull/108631 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Detect dangling assignment for "Container" case. (PR #108205)
@@ -601,17 +601,23 @@ void test() { std::optional o4 = std::optional(s); // FIXME: should work for assignment cases - v1 = {std::string()}; - o1 = std::string(); + v1 = {std::string()}; // expected-warning {{object backing the pointer}} + o1 = std::string(); // expected-warning {{object backing the pointer}} // no warning on copying pointers. std::vector n1 = {std::string_view()}; + n1 = {std::string_view()}; Xazax-hun wrote: Not strictly related to this PR, but we could add some tests for more deeply nested containers like `std::vector>`. Regardless if we gat warnings for those or not, documenting whether this case works in tests have some value. https://github.com/llvm/llvm-project/pull/108205 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang/AST] Make it possible to use SwiftAttr in type context (PR #108631)
@@ -6037,13 +6038,24 @@ class AttributedType : public Type, public llvm::FoldingSetNode { private: friend class ASTContext; // ASTContext creates these + const Attr *Attribute; + QualType ModifiedType; QualType EquivalentType; AttributedType(QualType canon, attr::Kind attrKind, QualType modified, QualType equivalent) + : AttributedType(canon, attrKind, nullptr, modified, equivalent) {} + + AttributedType(QualType canon, const Attr *attr, QualType modified, + QualType equivalent); + +private: + AttributedType(QualType canon, attr::Kind attrKind, const Attr *attr, + QualType modified, QualType equivalent) : Type(Attributed, canon, equivalent->getDependence()), -ModifiedType(modified), EquivalentType(equivalent) { +Attribute(attr), ModifiedType(modified), +EquivalentType(equivalent) { AttributedTypeBits.AttrKind = attrKind; Xazax-hun wrote: I see, thanks! In that case it makes sense to me to encode this invariant in the constructor as an assertion (either `attr` is null, or it has the same kind as `attrKind`). https://github.com/llvm/llvm-project/pull/108631 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang/AST] Make it possible to use SwiftAttr in type context (PR #108631)
@@ -7163,7 +7217,8 @@ static QualType rebuildAttributedTypeWithoutNullability(ASTContext &Ctx, Ctx, Attributed->getModifiedType()); assert(Modified.getTypePtr() != Attributed->getModifiedType().getTypePtr()); return Ctx.getAttributedType(Attributed->getAttrKind(), Modified, - Attributed->getEquivalentType()); + Attributed->getEquivalentType(), + Attributed->getAttr()); Xazax-hun wrote: The formatting looks off here. https://github.com/llvm/llvm-project/pull/108631 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang/AST] Make it possible to use SwiftAttr in type context (PR #108631)
@@ -6037,13 +6038,24 @@ class AttributedType : public Type, public llvm::FoldingSetNode { private: friend class ASTContext; // ASTContext creates these + const Attr *Attribute; + QualType ModifiedType; QualType EquivalentType; AttributedType(QualType canon, attr::Kind attrKind, QualType modified, QualType equivalent) + : AttributedType(canon, attrKind, nullptr, modified, equivalent) {} + + AttributedType(QualType canon, const Attr *attr, QualType modified, + QualType equivalent); + +private: + AttributedType(QualType canon, attr::Kind attrKind, const Attr *attr, + QualType modified, QualType equivalent) : Type(Attributed, canon, equivalent->getDependence()), -ModifiedType(modified), EquivalentType(equivalent) { +Attribute(attr), ModifiedType(modified), +EquivalentType(equivalent) { AttributedTypeBits.AttrKind = attrKind; Xazax-hun wrote: Do we need both `attrKind` and `attr`? It removed an indirection, but wondering if we should prefer that or memory. No strong feelings here. @erichkeane any opinions? https://github.com/llvm/llvm-project/pull/108631 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang/AST] Make it possible to use SwiftAttr in type context (PR #108631)
https://github.com/Xazax-hun approved this pull request. Overall looks good to me, I have one question and a couple nits inline. I think having access to Attr from AttributedType makes sense, I would not be surprised if this helped some other use cases like Lifetime annotations in Crubit. But please wait for a review from @erichkeane as well. https://github.com/llvm/llvm-project/pull/108631 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang/AST] Make it possible to use SwiftAttr in type context (PR #108631)
@@ -8732,6 +8786,11 @@ static void processTypeAttrs(TypeProcessingState &state, QualType &type, case ParsedAttr::AT_HLSLParamModifier: { HandleHLSLParamModifierAttr(state, type, attr, state.getSema()); attr.setUsedAsTypeAttr(); + break; + } Xazax-hun wrote: The formatting might be a bit off here. https://github.com/llvm/llvm-project/pull/108631 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang/AST] Make it possible to use SwiftAttr in type context (PR #108631)
@@ -7147,6 +7147,60 @@ static bool HandleWebAssemblyFuncrefAttr(TypeProcessingState &State, return false; } +static void HandleSwiftAttr(TypeProcessingState &State, TypeAttrLocation TAL, +QualType &QT, ParsedAttr &PAttr) { + if (TAL == TAL_DeclName) +return; + + Sema &S = State.getSema(); + auto &D = State.getDeclarator(); + + // If the attribute appears in declaration specifiers + // it should be handled as a declaration attribute, + // unless it's associated with a type or a function + // prototype (i.e. appears on a parameter or result type). + if (State.isProcessingDeclSpec()) { +if (!(D.isPrototypeContext() || + D.getContext() == DeclaratorContext::TypeName)) + return; + +if (auto *chunk = D.getInnermostNonParenChunk()) { + moveAttrFromListToList(PAttr, State.getCurrentAttributes(), + const_cast(chunk)->getAttrs()); + return; +} + } + + StringRef Str; + if (!S.checkStringLiteralArgumentAttr(PAttr, 0, Str)) { +PAttr.setInvalid(); +return; + } + + // If the attribute as attached to a paren move it closer to + // the declarator. This can happen in block declarations when + // an attribute is placed before `^` i.e. `(__attribute__((...)) ^)`. Xazax-hun wrote: I don't see a test case covering this scenario, should we add one? https://github.com/llvm/llvm-project/pull/108631 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang/AST] Make it possible to use SwiftAttr in type context (PR #108631)
https://github.com/Xazax-hun edited https://github.com/llvm/llvm-project/pull/108631 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Detect dangling assignment for "Container" case. (PR #108205)
https://github.com/Xazax-hun approved this pull request. LGTM! https://github.com/llvm/llvm-project/pull/108205 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland: [clang] Diagnose dangling issues for the "Container" case. #107213 (PR #108344)
https://github.com/Xazax-hun approved this pull request. LGTM! https://github.com/llvm/llvm-project/pull/108344 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Don't emit bogus dangling diagnostics when `[[gsl::Owner]]` and `[[clang::lifetimebound]]` are used together. (PR #108280)
@@ -300,6 +300,8 @@ Improvements to Clang's diagnostics - Clang now diagnoses cases where a dangling ``GSLOwner`` object is constructed, e.g. ``std::vector v = {std::string()};`` (#GH100526). +- Don't emit bogus dangling diagnostics when ``[[gsl::Owner]]`` and `[[clang::lifetimebound]]` are used together (#GH108272). Xazax-hun wrote: Do we fix a problem that was present in the previous release? Or is this something that was introduced since the branching? I think in case users of releases never saw these bogus warnings, there is no reason to mention this in the release notes. https://github.com/llvm/llvm-project/pull/108280 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Don't emit bogus dangling diagnostics when `[[gsl::Owner]]` and `[[clang::lifetimebound]]` are used together. (PR #108280)
https://github.com/Xazax-hun edited https://github.com/llvm/llvm-project/pull/108280 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Don't emit bogus dangling diagnostics when `[[gsl::Owner]]` and `[[clang::lifetimebound]]` are used together. (PR #108280)
https://github.com/Xazax-hun approved this pull request. LGTM! https://github.com/llvm/llvm-project/pull/108280 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Diagnose dangling issues for the "Container" case. (PR #107213)
https://github.com/Xazax-hun approved this pull request. LGTM! Thanks! https://github.com/llvm/llvm-project/pull/107213 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Respect the lifetimebound in assignment operator. (PR #106997)
https://github.com/Xazax-hun approved this pull request. After a couple more tests LGTM! https://github.com/llvm/llvm-project/pull/106997 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Respect the lifetimebound in assignment operator. (PR #106997)
@@ -287,3 +287,18 @@ std::span test2() { return abc; // expected-warning {{address of stack memory associated with local variable}} } } // namespace ctor_cases + +namespace GH106372 { +class [[gsl::Owner]] Foo {}; +class [[gsl::Pointer]] FooView {}; + +template +struct StatusOr { + template + StatusOr& operator=(U&& v [[clang::lifetimebound]]); +}; + +void test(StatusOr foo) { + foo = Foo(); // expected-warning {{object backing the pointer foo will be destroyed at the end}} Xazax-hun wrote: Oh, never mind. Actually, in that specialization people would not mark the argument of `operator=` lifetimebound. This makes total sense. Disregard this. https://github.com/llvm/llvm-project/pull/106997 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Respect the lifetimebound in assignment operator. (PR #106997)
@@ -287,3 +287,18 @@ std::span test2() { return abc; // expected-warning {{address of stack memory associated with local variable}} } } // namespace ctor_cases + +namespace GH106372 { +class [[gsl::Owner]] Foo {}; +class [[gsl::Pointer]] FooView {}; + +template +struct StatusOr { + template + StatusOr& operator=(U&& v [[clang::lifetimebound]]); +}; + +void test(StatusOr foo) { + foo = Foo(); // expected-warning {{object backing the pointer foo will be destroyed at the end}} Xazax-hun wrote: I am actually a bit split on this code snippet. I think this is probably fine in most cases, but I wonder if this is prone to false positives. We only know that `FooView` is a pointer, but strictly speaking we have no idea what `StatusOr` is doing. We could have an explicit specialization for `StatusOr` that actually stores a `Foo` inside instead of `FooView`. Is that good practice? Absolutely not. I am just wondering if this is justified enough for an on by default warning. (Same goes for the initialization case, not just the assignment.) https://github.com/llvm/llvm-project/pull/106997 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Respect the lifetimebound in assignment operator. (PR #106997)
@@ -968,6 +972,23 @@ static bool pathOnlyHandlesGslPointer(IndirectLocalPath &Path) { return false; } +static bool isAssginmentOperatorLifetimeBound(CXXMethodDecl *CMD) { + if (!CMD) +return false; + assert(CMD->getOverloadedOperator() == OverloadedOperatorKind::OO_Equal); Xazax-hun wrote: What about compound assignment operators? Do you plan to support those in a follow-up? That being said, I'd love to see a test case just to make sure this assert is not triggered. https://github.com/llvm/llvm-project/pull/106997 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Fix nullptr dereference for symbols from pointer invalidation (PR #106568)
https://github.com/Xazax-hun approved this pull request. LGTM! https://github.com/llvm/llvm-project/pull/106568 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Moving TaintPropagation checker out of alpha (PR #67352)
Xazax-hun wrote: I have no concerns with moving forward here, my understanding is that the blockers have been resolved. Moreover, we are early in the development cycle for the next release so we still have a lot of time to get more experience with this check once it is move out of alpha. But I'd also wait for Artem to greenlight this. https://github.com/llvm/llvm-project/pull/67352 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer][NFC] Remove a non-actionable dump (PR #106232)
https://github.com/Xazax-hun approved this pull request. LGTM! https://github.com/llvm/llvm-project/pull/106232 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Detect leaks of stack addresses via output params, indirect globals 3/3 (PR #105648)
@@ -348,12 +357,27 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS, ExplodedNode *Node = Ctx.getPredecessor(); + bool ExitingTopFrame = + Ctx.getPredecessor()->getLocationContext()->inTopFrame(); + + if (ExitingTopFrame && Node->getLocation().getTag() && + Node->getLocation().getTag()->getTagDescription() == Xazax-hun wrote: I am happy with the new version. https://github.com/llvm/llvm-project/pull/105648 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Detect leaks of stack addresses via output params, indirect globals 3/3 (PR #105648)
@@ -348,12 +357,27 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS, ExplodedNode *Node = Ctx.getPredecessor(); + bool ExitingTopFrame = + Ctx.getPredecessor()->getLocationContext()->inTopFrame(); + + if (ExitingTopFrame && Node->getLocation().getTag() && + Node->getLocation().getTag()->getTagDescription() == Xazax-hun wrote: Is this a common idiom to check for the tag description? I wonder if we could come up with something that is less stringly typed. https://github.com/llvm/llvm-project/pull/105648 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Detect leak of a stack address through output arguments 2/3 (PR #105653)
@@ -297,20 +314,31 @@ std::optional printReferrer(const MemRegion *Referrer) { return "global"; assert(isa(Space)); return "stack"; - }(Referrer->getMemorySpace()); - - // We should really only have VarRegions here. - // Anything else is really surprising, and we should get notified if such - // ever happens. - const auto *ReferrerVar = dyn_cast(Referrer); - if (!ReferrerVar) { -assert(false && "We should have a VarRegion here"); -return std::nullopt; // Defensively skip this one. + }(getStackOrGlobalSpaceRegion(Referrer)); + + while (!Referrer->canPrintPretty()) { +if (const auto *SymReg = dyn_cast(Referrer); +SymReg && SymReg->getSymbol()->getOriginRegion()) { + Referrer = SymReg->getSymbol()->getOriginRegion()->getBaseRegion(); +} else if (isa(Referrer)) { + // Skip members of a class, it is handled in CheckExprLifetime.cpp as + // warn_bind_ref_member_to_parameter or + // warn_init_ptr_member_to_parameter_addr + return std::nullopt; +} else { + Referrer->dump(); Xazax-hun wrote: I think these dumps are not actionable to end users and we should not expose this. https://github.com/llvm/llvm-project/pull/105653 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Remove the EnableLifetimeWarnings flag in lifetime analysis. (PR #105884)
https://github.com/Xazax-hun approved this pull request. Thanks, this is a nice cleanup, but I'd prefer either the PR to be split up, or renamed to emphasize the bug fix aspect of the PR. https://github.com/llvm/llvm-project/pull/105884 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Remove the EnableLifetimeWarnings flag in lifetime analysis. (PR #105884)
@@ -393,11 +392,9 @@ static void visitFunctionCallArguments(IndirectLocalPath &Path, Expr *Call, Path.push_back({IndirectLocalPathEntry::LifetimeBoundCall, Arg, D}); if (Arg->isGLValue()) visitLocalsRetainedByReferenceBinding(Path, Arg, RK_ReferenceBinding, -Visit, -/*EnableLifetimeWarnings=*/false); +Visit); else - visitLocalsRetainedByInitializer(Path, Arg, Visit, true, - /*EnableLifetimeWarnings=*/false); Xazax-hun wrote: I think the PR needs to be renamed. From a PR with the title `Remove the EnableLifetimeWarnings flag in lifetime analysis.` I'd expect either an NFC or removing a feature, not an actual bug fix. I think we should either split this up in two PRs, or rename the title to describe the bugfix aspect. https://github.com/llvm/llvm-project/pull/105884 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer][NFC] Add tests for and refactor StackAddrEscapeChecker 1/3 (PR #105652)
@@ -374,13 +395,13 @@ void StackAddrEscapeChecker::checkEndFunction(const ReturnStmt *RS, // Generate a report for this bug. const StringRef CommonSuffix = -"upon returning to the caller. This will be a dangling reference"; +" upon returning to the caller. This will be a dangling reference"; Xazax-hun wrote: Is having two spaces after a dot a common convention we have? This is not a change in this PR, I'm just wondering if this was intentional or a typo. https://github.com/llvm/llvm-project/pull/105652 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Limit `isTainted()` by skipping complicated symbols (PR #105493)
https://github.com/Xazax-hun approved this pull request. LGTM! https://github.com/llvm/llvm-project/pull/105493 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Merge lifetimebound and GSL code paths for lifetime analysis (PR #104906)
https://github.com/Xazax-hun edited https://github.com/llvm/llvm-project/pull/104906 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Merge lifetimebound and GSL code paths for lifetime analysis (PR #104906)
@@ -478,13 +449,32 @@ static void visitLifetimeBoundArguments(IndirectLocalPath &Path, Expr *Call, CheckCoroObjArg = false; if (implicitObjectParamIsLifetimeBound(Callee) || CheckCoroObjArg) VisitLifetimeBoundArg(Callee, ObjectArg); +else if (EnableLifetimeWarnings) { + if (auto *CME = dyn_cast(Callee); + CME && shouldTrackImplicitObjectArg(CME)) +VisitGSLPointerArg(Callee, ObjectArg, +!Callee->getReturnType()->isReferenceType()); +} } for (unsigned I = 0, N = std::min(Callee->getNumParams(), Args.size()); I != N; ++I) { if (CheckCoroCall || Callee->getParamDecl(I)->hasAttr()) VisitLifetimeBoundArg(Callee->getParamDecl(I), Args[I]); +else if (EnableLifetimeWarnings) { + if (I == 0) { Xazax-hun wrote: Nit: maybe `EnableLifetimeWarnings && I == 0` to reduce the indentation? https://github.com/llvm/llvm-project/pull/104906 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Merge lifetimebound and GSL code paths for lifetime analysis (PR #104906)
https://github.com/Xazax-hun approved this pull request. LGTM, thanks! https://github.com/llvm/llvm-project/pull/104906 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Merge lifetimebound and GSL code paths for lifetime analysis (PR #104906)
https://github.com/Xazax-hun edited https://github.com/llvm/llvm-project/pull/104906 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Emit -Wdangling diagnoses for cases where a gsl-pointer is construct from a gsl-owner object in a container. (PR #104556)
https://github.com/Xazax-hun approved this pull request. LGTM, thanks for figuring this out! https://github.com/llvm/llvm-project/pull/104556 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Revert "[clang][driver] Fix -print-target-triple OS version for apple targets" (PR #104563)
https://github.com/Xazax-hun approved this pull request. LGTM! https://github.com/llvm/llvm-project/pull/104563 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Fix -print-target-triple OS version for apple targets (PR #104037)
Xazax-hun wrote: Hey, thanks for letting me know. Feel free to revert this for now! https://github.com/llvm/llvm-project/pull/104037 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Fix -print-target-triple OS version for apple targets (PR #104037)
https://github.com/Xazax-hun closed https://github.com/llvm/llvm-project/pull/104037 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Fix -print-target-triple OS version for apple targets (PR #104037)
https://github.com/Xazax-hun ready_for_review https://github.com/llvm/llvm-project/pull/104037 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Fix -print-target-triple OS version for apple targets (PR #104037)
https://github.com/Xazax-hun updated https://github.com/llvm/llvm-project/pull/104037 From 3cb5a4539962fbdf11ccc07b9b1a8fa3bf2fec9a Mon Sep 17 00:00:00 2001 From: Gabor Horvath Date: Wed, 14 Aug 2024 16:17:40 +0100 Subject: [PATCH] Fix -print-target-triple OS version for apple targets The target needs to be initialized in order to computer the correct target triple from the command line. Without initialized targets the OS component of the tripple might not reflect what would be computer by the driver for an actual compiler invocation. --- clang/lib/Driver/Driver.cpp | 13 -- .../test/Driver/darwin-print-target-triple.c | 42 +++ 2 files changed, 52 insertions(+), 3 deletions(-) create mode 100644 clang/test/Driver/darwin-print-target-triple.c diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index e12416e51f8d24..5b95019c25cab6 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -2271,8 +2271,7 @@ bool Driver::HandleImmediateArgs(Compilation &C) { return false; } - if (C.getArgs().hasArg(options::OPT_print_libgcc_file_name)) { -ToolChain::RuntimeLibType RLT = TC.GetRuntimeLibType(C.getArgs()); + auto initializeTargets = [&]() { const llvm::Triple Triple(TC.ComputeEffectiveClangTriple(C.getArgs())); // The 'Darwin' toolchain is initialized only when its arguments are // computed. Get the default arguments for OFK_None to ensure that @@ -2282,6 +2281,12 @@ bool Driver::HandleImmediateArgs(Compilation &C) { // FIXME: For some more esoteric targets the default toolchain is not the //correct one. C.getArgsForToolChain(&TC, Triple.getArchName(), Action::OFK_None); +return Triple; + }; + + if (C.getArgs().hasArg(options::OPT_print_libgcc_file_name)) { +ToolChain::RuntimeLibType RLT = TC.GetRuntimeLibType(C.getArgs()); +const llvm::Triple Triple = initializeTargets(); RegisterEffectiveTriple TripleRAII(TC, Triple); switch (RLT) { case ToolChain::RLT_CompilerRT: @@ -2325,7 +2330,9 @@ bool Driver::HandleImmediateArgs(Compilation &C) { } if (C.getArgs().hasArg(options::OPT_print_target_triple)) { -llvm::outs() << TC.getTripleString() << "\n"; +initializeTargets(); +llvm::Triple Triple(TC.ComputeEffectiveClangTriple(C.getArgs())); +llvm::outs() << Triple.getTriple() << "\n"; return false; } diff --git a/clang/test/Driver/darwin-print-target-triple.c b/clang/test/Driver/darwin-print-target-triple.c new file mode 100644 index 00..4f5fdfe9d0db34 --- /dev/null +++ b/clang/test/Driver/darwin-print-target-triple.c @@ -0,0 +1,42 @@ +// Test the output of -print-target-triple on Darwin. +// See https://github.com/llvm/llvm-project/issues/61762 + +// +// All platforms +// + +// RUN: %clang -print-target-triple \ +// RUN: --target=x86_64-apple-macos -mmacos-version-min=15 \ +// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-MACOS %s +// CHECK-CLANGRT-MACOS: x86_64-apple-macosx15.0.0 + +// RUN: %clang -print-target-triple \ +// RUN: --target=arm64-apple-ios -mios-version-min=9 \ +// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-IOS %s +// CHECK-CLANGRT-IOS: arm64-apple-ios9.0.0 + +// RUN: %clang -print-target-triple \ +// RUN: --target=arm64-apple-watchos -mwatchos-version-min=3 \ +// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-WATCHOS %s +// CHECK-CLANGRT-WATCHOS: arm64-apple-watchos3.0.0 + +// RUN: %clang -print-target-triple \ +// RUN: --target=armv7k-apple-watchos -mwatchos-version-min=3 \ +// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-WATCHOS-ARMV7K %s +// CHECK-CLANGRT-WATCHOS-ARMV7K: thumbv7-apple-watchos3.0.0 + +// RUN: %clang -print-target-triple \ +// RUN: --target=arm64-apple-tvos -mtvos-version-min=1\ +// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-TVOS %s +// CHECK-CLANGRT-TVOS: arm64-apple-tvos1.0.0 + +// RUN: %clang -print-target-triple \ +// RUN: --target=arm64-apple-driverkit \ +// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-DRIVERKIT %s +// CHECK-CLANGRT-DRIVERKIT: arm64-apple-driverkit19.0.0 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Fix -print-target-triple OS version for apple targets (PR #104037)
https://github.com/Xazax-hun edited https://github.com/llvm/llvm-project/pull/104037 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Fix -print-target-triple OS version for apple targets (PR #104037)
https://github.com/Xazax-hun created https://github.com/llvm/llvm-project/pull/104037 The target needs to be initialized in order to computer the correct target triple from the command line. Without initialized targets the OS component of the tripple might not reflect what would be computer by the driver for an actual compiler invocation. From 02ee150e3fb3017ee55d73f4254b8a498cc85ada Mon Sep 17 00:00:00 2001 From: Gabor Horvath Date: Wed, 14 Aug 2024 16:17:40 +0100 Subject: [PATCH] Fix -print-target-triple OS version for apple targets The target needs to be initialized in order to computer the correct target triple from the command line. Without initialized targets the OS component of the tripple might not reflect what would be computer by the driver for an actual compiler invocation. --- clang/lib/Driver/Driver.cpp | 13 +-- .../test/Driver/darwin-print-target-triple.c | 35 +++ 2 files changed, 45 insertions(+), 3 deletions(-) create mode 100644 clang/test/Driver/darwin-print-target-triple.c diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index e12416e51f8d24..5b95019c25cab6 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -2271,8 +2271,7 @@ bool Driver::HandleImmediateArgs(Compilation &C) { return false; } - if (C.getArgs().hasArg(options::OPT_print_libgcc_file_name)) { -ToolChain::RuntimeLibType RLT = TC.GetRuntimeLibType(C.getArgs()); + auto initializeTargets = [&]() { const llvm::Triple Triple(TC.ComputeEffectiveClangTriple(C.getArgs())); // The 'Darwin' toolchain is initialized only when its arguments are // computed. Get the default arguments for OFK_None to ensure that @@ -2282,6 +2281,12 @@ bool Driver::HandleImmediateArgs(Compilation &C) { // FIXME: For some more esoteric targets the default toolchain is not the //correct one. C.getArgsForToolChain(&TC, Triple.getArchName(), Action::OFK_None); +return Triple; + }; + + if (C.getArgs().hasArg(options::OPT_print_libgcc_file_name)) { +ToolChain::RuntimeLibType RLT = TC.GetRuntimeLibType(C.getArgs()); +const llvm::Triple Triple = initializeTargets(); RegisterEffectiveTriple TripleRAII(TC, Triple); switch (RLT) { case ToolChain::RLT_CompilerRT: @@ -2325,7 +2330,9 @@ bool Driver::HandleImmediateArgs(Compilation &C) { } if (C.getArgs().hasArg(options::OPT_print_target_triple)) { -llvm::outs() << TC.getTripleString() << "\n"; +initializeTargets(); +llvm::Triple Triple(TC.ComputeEffectiveClangTriple(C.getArgs())); +llvm::outs() << Triple.getTriple() << "\n"; return false; } diff --git a/clang/test/Driver/darwin-print-target-triple.c b/clang/test/Driver/darwin-print-target-triple.c new file mode 100644 index 00..84f7832b83f478 --- /dev/null +++ b/clang/test/Driver/darwin-print-target-triple.c @@ -0,0 +1,35 @@ +// Test the output of -print-target-triple on Darwin. + +// +// All platforms +// + +// RUN: %clang -print-target-triple \ +// RUN: --target=x86_64-apple-macos -mmacos-version-min=15 \ +// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-MACOS %s +// CHECK-CLANGRT-MACOS: x86_64-apple-macosx15.0.0 + +// RUN: %clang -print-target-triple \ +// RUN: --target=arm64-apple-ios -mios-version-min=9 \ +// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-IOS %s +// CHECK-CLANGRT-IOS: arm64-apple-ios9.0.0 + +// RUN: %clang -print-target-triple \ +// RUN: --target=arm64-apple-watchos -mwatchos-version-min=3 \ +// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-WATCHOS %s +// CHECK-CLANGRT-WATCHOS: arm64-apple-watchos3.0.0 + +// RUN: %clang -print-target-triple \ +// RUN: --target=arm64-apple-tvos -mtvos-version-min=1\ +// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-TVOS %s +// CHECK-CLANGRT-TVOS: arm64-apple-tvos1.0.0 + +// RUN: %clang -print-target-triple \ +// RUN: --target=arm64-apple-driverkit \ +// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-DRIVERKIT %s +// CHECK-CLANGRT-DRIVERKIT: arm64-apple-driverkit19.0.0 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add lifetimebound attr to std::span/std::string_view constructor (PR #103716)
https://github.com/Xazax-hun approved this pull request. Looks great, thanks! https://github.com/llvm/llvm-project/pull/103716 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add lifetimebound attr to std::span/std::string_view constructor (PR #103716)
@@ -216,6 +216,59 @@ void Sema::inferGslOwnerPointerAttribute(CXXRecordDecl *Record) { inferGslPointerAttribute(Record, Record); } +void Sema::inferLifetimeBoundAttribute(FunctionDecl *FD) { + if (FD->getNumParams() == 0) +return; + + if (unsigned BuiltinID = FD->getBuiltinID()) { +// Add lifetime attribute to std::move, std::fowrard et al. +switch (BuiltinID) { +case Builtin::BIaddressof: +case Builtin::BI__addressof: +case Builtin::BI__builtin_addressof: +case Builtin::BIas_const: Xazax-hun wrote: I start to have the feeling we are writing code like this over and over again. Some examples: https://github.com/llvm/llvm-project/blob/df57833ea8a3f527b7941576b4a130ddd4361e61/clang/lib/Analysis/BodyFarm.cpp#L718 https://github.com/llvm/llvm-project/blob/df57833ea8a3f527b7941576b4a130ddd4361e61/clang/lib/AST/ExprConstant.cpp#L8763 But there are more, and I always wonder if they can get out of sync. I wonder if this is time to introduce some helper functions that can be reused. https://github.com/llvm/llvm-project/pull/103716 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add lifetimebound attr to std::span/std::string_view constructor (PR #103716)
https://github.com/Xazax-hun edited https://github.com/llvm/llvm-project/pull/103716 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Fix casting in `ChromiumCheckModel`. (PR #101640)
https://github.com/Xazax-hun approved this pull request. https://github.com/llvm/llvm-project/pull/101640 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Fix bug in `buildContainsExprConsumedInDifferentBlock()`. (PR #100874)
https://github.com/Xazax-hun approved this pull request. LGTM! https://github.com/llvm/llvm-project/pull/100874 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Keep alive short-circuiting condition subexpressions in a conditional (PR #100745)
https://github.com/Xazax-hun commented: Nice catch, thanks for fixing! https://github.com/llvm/llvm-project/pull/100745 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Enable the -Wdangling-assignment-gsl diagnostic by default. (PR #100708)
https://github.com/Xazax-hun approved this pull request. LGTM! https://github.com/llvm/llvm-project/pull/100708 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix lifetimebound for field access (PR #100197)
Xazax-hun wrote: Looks like cherry-pick failed and this needs manual back porting. https://github.com/llvm/llvm-project/pull/100197 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Model builtin-like functions as builtin functions (PR #99886)
https://github.com/Xazax-hun approved this pull request. LGTM, thanks! https://github.com/llvm/llvm-project/pull/99886 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Disallow applying `onwership_returns` to functions that return non-pointers (PR #99564)
Xazax-hun wrote: This looks good to me as far as the current use/behavior is concerned. That being said, I think these annotations make sense to some non-pointer types as well, e.g., integer handles to other system resources. I did not find any test/functionality for this scenario, so I am OK with this change for now, but this is something that we want to refine if new functionality is introduced in the future. https://github.com/llvm/llvm-project/pull/99564 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] NFC, simplify the code in CheckExprLifetime.cpp (PR #99637)
https://github.com/Xazax-hun approved this pull request. LGTM! https://github.com/llvm/llvm-project/pull/99637 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Fix -print-libgcc-file-name on Darwin platforms (PR #98325)
https://github.com/Xazax-hun closed https://github.com/llvm/llvm-project/pull/98325 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add `std::span` to the default gsl pointer annotation list. (PR #99622)
Xazax-hun wrote: Hmm. Can/should we also make these const? https://github.com/llvm/llvm-project/pull/99622 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add `std::span` to the default gsl pointer annotation list. (PR #99622)
https://github.com/Xazax-hun approved this pull request. LGTM! https://github.com/llvm/llvm-project/pull/99622 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Fix -print-libgcc-file-name on Darwin platforms (PR #98325)
Xazax-hun wrote: It has been a couple of days without any activity. I plan to merge this PR tomorrow and address any potential concerns in a followup PR. Let me know if you want me to wait a bit more if there are any concerns that should be addressed first. https://github.com/llvm/llvm-project/pull/98325 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Extend lifetime analysis to support assignments for pointer-like objects. (PR #99032)
Xazax-hun wrote: Any reason why it is not enabled by default for clang-19? Are there any known false positives or just precautions? https://github.com/llvm/llvm-project/pull/99032 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Bounds-Safety] Add sized_by, counted_by_or_null & sized_by_or_null (PR #93231)
Xazax-hun wrote: I really like the test coverage of this feature, but I started to wonder if we should start splitting up some sema tests into subfolders. I think the bounds-safety related tests would be a good candidate to have their own subfolder :) https://github.com/llvm/llvm-project/pull/93231 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Refactor: Introduce a new LifetimeKind for the assignment case, NFC (PR #99005)
https://github.com/Xazax-hun approved this pull request. LGTM! https://github.com/llvm/llvm-project/pull/99005 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Fix -print-libgcc-file-name on Darwin platforms (PR #98325)
Xazax-hun wrote: @MaskRay Thanks for the review. Do you think this is good to go or is there anything else you want me to change? https://github.com/llvm/llvm-project/pull/98325 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] reland (PR #98896)
https://github.com/Xazax-hun approved this pull request. LGTM! https://github.com/llvm/llvm-project/pull/98896 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow]Propagate the result object location for CXXDefaultInitExpr. (PR #98490)
https://github.com/Xazax-hun closed https://github.com/llvm/llvm-project/pull/98490 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow]Propagate the result object location for CXXDefaultInitExpr. (PR #98490)
https://github.com/Xazax-hun approved this pull request. LGTM, thanks! https://github.com/llvm/llvm-project/pull/98490 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Fix -print-libgcc-file-name on Darwin platforms (PR #98325)
https://github.com/Xazax-hun updated https://github.com/llvm/llvm-project/pull/98325 From 200d0b7ab622012fb8e74fcc40df6ba3be6bc215 Mon Sep 17 00:00:00 2001 From: Gabor Horvath Date: Mon, 8 Jul 2024 11:18:02 +0100 Subject: [PATCH] [clang][driver] Fix --print-libgcc-file-name on Darwin platforms On Darwin, --print-libgcc-file-name was returning a nonsensical result. It would return the name of the library that would be used by the default toolchain implementation, but that was something that didn't exist on Darwin. Fixing this requires initializing the Darwin toolchain before processing `--print-libgcc-file-name`. Previously, the Darwin toolchain would only be initialized when building the jobs for this compilation, which is too late since `--print-libgcc-file-name` requires the toolchain to be initialized in order to provide the right results. rdar://90633749 --- clang/include/clang/Driver/Driver.h | 5 +- clang/lib/Driver/Driver.cpp | 10 +- clang/lib/Driver/ToolChains/Darwin.cpp| 65 - clang/lib/Driver/ToolChains/Darwin.h | 13 +++ .../Driver/darwin-print-libgcc-file-name.c| 91 +++ 5 files changed, 160 insertions(+), 24 deletions(-) create mode 100644 clang/test/Driver/darwin-print-libgcc-file-name.c diff --git a/clang/include/clang/Driver/Driver.h b/clang/include/clang/Driver/Driver.h index cc1538372d5f8..04b46782467d6 100644 --- a/clang/include/clang/Driver/Driver.h +++ b/clang/include/clang/Driver/Driver.h @@ -628,8 +628,9 @@ class Driver { /// treated before building actions or binding tools. /// /// \return Whether any compilation should be built for this - /// invocation. - bool HandleImmediateArgs(const Compilation &C); + /// invocation. The compilation can only be modified when + /// this function returns false. + bool HandleImmediateArgs(Compilation &C); /// ConstructAction - Construct the appropriate action to do for /// \p Phase on the \p Input, taking in to account arguments diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index 221e222bdd47d..e6664326884d8 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -2128,7 +2128,7 @@ void Driver::HandleAutocompletions(StringRef PassedFlags) const { llvm::outs() << llvm::join(SuggestedCompletions, "\n") << '\n'; } -bool Driver::HandleImmediateArgs(const Compilation &C) { +bool Driver::HandleImmediateArgs(Compilation &C) { // The order these options are handled in gcc is all over the place, but we // don't expect inconsistencies w.r.t. that to matter in practice. @@ -2271,6 +2271,14 @@ bool Driver::HandleImmediateArgs(const Compilation &C) { if (C.getArgs().hasArg(options::OPT_print_libgcc_file_name)) { ToolChain::RuntimeLibType RLT = TC.GetRuntimeLibType(C.getArgs()); const llvm::Triple Triple(TC.ComputeEffectiveClangTriple(C.getArgs())); +// The 'Darwin' toolchain is initialized only when its arguments are +// computed. Get the default arguments for OFK_None to ensure that +// initialization is performed before trying to access properties of +// the toolchain in the functions below. +// FIXME: Remove when darwin's toolchain is initialized during construction. +// FIXME: For some more esoteric targets the default toolchain is not the +//correct one. +C.getArgsForToolChain(&TC, Triple.getArchName(), Action::OFK_None); RegisterEffectiveTriple TripleRAII(TC, Triple); switch (RLT) { case ToolChain::RLT_CompilerRT: diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index f354b0974d5f2..c6f9d7beffb1d 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -1272,23 +1272,8 @@ unsigned DarwinClang::GetDefaultDwarfVersion() const { void MachO::AddLinkRuntimeLib(const ArgList &Args, ArgStringList &CmdArgs, StringRef Component, RuntimeLinkOptions Opts, bool IsShared) const { - SmallString<64> DarwinLibName = StringRef("libclang_rt."); - // On Darwin the builtins component is not in the library name. - if (Component != "builtins") { -DarwinLibName += Component; -if (!(Opts & RLO_IsEmbedded)) - DarwinLibName += "_"; - } - - DarwinLibName += getOSLibraryNameSuffix(); - DarwinLibName += IsShared ? "_dynamic.dylib" : ".a"; - SmallString<128> Dir(getDriver().ResourceDir); - llvm::sys::path::append(Dir, "lib", "darwin"); - if (Opts & RLO_IsEmbedded) -llvm::sys::path::append(Dir, "macho_embedded"); - - SmallString<128> P(Dir); - llvm::sys::path::append(P, DarwinLibName); + std::string P = getCompilerRT( + Args, Component, IsShared ? ToolChain::FT_Shared : ToolChain::FT_Static); // For now, allow missing resource libraries to support developers who may // not have compiler-rt checked out or integrated into their build (unless @@ -1303,18 +128
[clang] [clang][driver] Fix -print-libgcc-file-name on Darwin platforms (PR #98325)
@@ -0,0 +1,27 @@ +// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name \ +// RUN: --target=armv7em-apple-darwin \ +// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-HARD_STATIC %s +// CHECK-CLANGRT-HARD_STATIC: libclang_rt.hard_static.a + +// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name \ +// RUN: --target=armv7em-apple-darwin -msoft-float \ +// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-SOFT_STATIC %s +// CHECK-CLANGRT-SOFT_STATIC: libclang_rt.soft_static.a + +// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name \ +// RUN: --target=armv7em-apple-darwin -fPIC \ +// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-HARD_PIC %s +// CHECK-CLANGRT-HARD_PIC: libclang_rt.hard_pic.a + +// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name \ +// RUN: --target=armv7em-apple-darwin -msoft-float -fPIC \ +// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-SOFT_PIC %s +// CHECK-CLANGRT-SOFT_PIC: libclang_rt.soft_pic.a + +// FIXME: -print-libgcc-file-name is using the default toolchain +//so the tests above do not give the right answer yet. +// XFAIL: * Xazax-hun wrote: Not in the immediate future, deleting the test. https://github.com/llvm/llvm-project/pull/98325 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Fix -print-libgcc-file-name on Darwin platforms (PR #98325)
@@ -2230,6 +2239,7 @@ bool Driver::HandleImmediateArgs(const Compilation &C) { } if (C.getArgs().hasArg(options::OPT_print_runtime_dir)) { +initDarwinTarget(); Xazax-hun wrote: I removed this change for now because I think we should probably fix this separately. Only adding the early init did not change the behavior for the cases we are interested in. https://github.com/llvm/llvm-project/pull/98325 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Fix -print-libgcc-file-name on Darwin platforms (PR #98325)
https://github.com/Xazax-hun updated https://github.com/llvm/llvm-project/pull/98325 From 9f3c99e278193af99d6a8928612f517be95aa6a6 Mon Sep 17 00:00:00 2001 From: Gabor Horvath Date: Mon, 8 Jul 2024 11:18:02 +0100 Subject: [PATCH] Fix --print-libgcc-file-name on Darwin platforms On Darwin, --print-libgcc-file-name was returning a nonsensical result. It would return the name of the library that would be used by the default toolchain implementation, but that was something that didn't exist on Darwin. Fixing this requires initializing the Darwin toolchain before processing `--print-libgcc-file-name`. Previously, the Darwin toolchain would only be initialized when building the jobs for this compilation, which is too late since `--print-libgcc-file-name` requires the toolchain to be initialized in order to provide the right results. rdar://90633749 --- clang/include/clang/Driver/Driver.h | 5 +- clang/lib/Driver/Driver.cpp | 8 +- clang/lib/Driver/ToolChains/Darwin.cpp| 65 - clang/lib/Driver/ToolChains/Darwin.h | 13 +++ .../darwin-embedded-print-libgcc-file-name.c | 27 ++ .../Driver/darwin-print-libgcc-file-name.c| 91 +++ 6 files changed, 185 insertions(+), 24 deletions(-) create mode 100644 clang/test/Driver/darwin-embedded-print-libgcc-file-name.c create mode 100644 clang/test/Driver/darwin-print-libgcc-file-name.c diff --git a/clang/include/clang/Driver/Driver.h b/clang/include/clang/Driver/Driver.h index cc1538372d5f8..04b46782467d6 100644 --- a/clang/include/clang/Driver/Driver.h +++ b/clang/include/clang/Driver/Driver.h @@ -628,8 +628,9 @@ class Driver { /// treated before building actions or binding tools. /// /// \return Whether any compilation should be built for this - /// invocation. - bool HandleImmediateArgs(const Compilation &C); + /// invocation. The compilation can only be modified when + /// this function returns false. + bool HandleImmediateArgs(Compilation &C); /// ConstructAction - Construct the appropriate action to do for /// \p Phase on the \p Input, taking in to account arguments diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index 221e222bdd47d..cb1bc7e36d39d 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -2128,7 +2128,7 @@ void Driver::HandleAutocompletions(StringRef PassedFlags) const { llvm::outs() << llvm::join(SuggestedCompletions, "\n") << '\n'; } -bool Driver::HandleImmediateArgs(const Compilation &C) { +bool Driver::HandleImmediateArgs(Compilation &C) { // The order these options are handled in gcc is all over the place, but we // don't expect inconsistencies w.r.t. that to matter in practice. @@ -2271,6 +2271,12 @@ bool Driver::HandleImmediateArgs(const Compilation &C) { if (C.getArgs().hasArg(options::OPT_print_libgcc_file_name)) { ToolChain::RuntimeLibType RLT = TC.GetRuntimeLibType(C.getArgs()); const llvm::Triple Triple(TC.ComputeEffectiveClangTriple(C.getArgs())); +// The 'Darwin' toolchain is initialized only when its arguments are +// computed. Get the default arguments for OFK_None to ensure that +// initialization is performed before trying to access properties of +// the toolchain in the functions below. +// FIXME: Remove when darwin's toolchain is initialized during construction. +C.getArgsForToolChain(&TC, Triple.getArchName(), Action::OFK_None); RegisterEffectiveTriple TripleRAII(TC, Triple); switch (RLT) { case ToolChain::RLT_CompilerRT: diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index f354b0974d5f2..c6f9d7beffb1d 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -1272,23 +1272,8 @@ unsigned DarwinClang::GetDefaultDwarfVersion() const { void MachO::AddLinkRuntimeLib(const ArgList &Args, ArgStringList &CmdArgs, StringRef Component, RuntimeLinkOptions Opts, bool IsShared) const { - SmallString<64> DarwinLibName = StringRef("libclang_rt."); - // On Darwin the builtins component is not in the library name. - if (Component != "builtins") { -DarwinLibName += Component; -if (!(Opts & RLO_IsEmbedded)) - DarwinLibName += "_"; - } - - DarwinLibName += getOSLibraryNameSuffix(); - DarwinLibName += IsShared ? "_dynamic.dylib" : ".a"; - SmallString<128> Dir(getDriver().ResourceDir); - llvm::sys::path::append(Dir, "lib", "darwin"); - if (Opts & RLO_IsEmbedded) -llvm::sys::path::append(Dir, "macho_embedded"); - - SmallString<128> P(Dir); - llvm::sys::path::append(P, DarwinLibName); + std::string P = getCompilerRT( + Args, Component, IsShared ? ToolChain::FT_Shared : ToolChain::FT_Static); // For now, allow missing resource libraries to support developers who may // not have compiler-rt checked out or integrated into their build (unless @@
[clang] [clang][driver] Fix -print-libgcc-file-name on Darwin platforms (PR #98325)
@@ -1303,18 +1288,55 @@ void MachO::AddLinkRuntimeLib(const ArgList &Args, ArgStringList &CmdArgs, // rpaths. This is currently true from this place, but we need to be // careful if this function is ever called before user's rpaths are emitted. if (Opts & RLO_AddRPath) { -assert(DarwinLibName.ends_with(".dylib") && "must be a dynamic library"); +assert(StringRef(P).ends_with(".dylib") && "must be a dynamic library"); // Add @executable_path to rpath to support having the dylib copied with // the executable. CmdArgs.push_back("-rpath"); CmdArgs.push_back("@executable_path"); -// Add the path to the resource dir to rpath to support using the dylib -// from the default location without copying. +// Add the compiler-rt library's directory to rpath to support using the +// dylib from the default location without copying. CmdArgs.push_back("-rpath"); -CmdArgs.push_back(Args.MakeArgString(Dir)); +CmdArgs.push_back(Args.MakeArgString(llvm::sys::path::parent_path(P))); + } +} + +std::string MachO::getCompilerRT(const ArgList &, StringRef Component, + FileType Type) const { + assert(Type != ToolChain::FT_Object && + "it doesn't make sense to ask for the compiler-rt library name as an " + "object file"); + SmallString<64> MachOLibName = StringRef("libclang_rt"); + // On MachO, the builtins component is not in the library name + if (Component != "builtins") { +MachOLibName += '.'; +MachOLibName += Component; + } + MachOLibName += Type == ToolChain::FT_Shared ? "_dynamic.dylib" : ".a"; Xazax-hun wrote: While `-print-libgcc-file-name` can only output the `.a` version, there are other callers of this API that query the dylib. https://github.com/llvm/llvm-project/pull/98325 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][driver] Fix -print-libgcc-file-name on Darwin platforms (PR #98325)
@@ -356,6 +363,12 @@ class LLVM_LIBRARY_VISIBILITY Darwin : public MachO { void addProfileRTLibs(const llvm::opt::ArgList &Args, llvm::opt::ArgStringList &CmdArgs) const override; + // Return the full path of the compiler-rt library on a Darwin MachO system. + // Those are under /lib/darwin/<...>(.dylib|.a). Xazax-hun wrote: While `-print-libgcc-file-name` can only output the `.a` version, there are other callers of this API that query the dylib. https://github.com/llvm/llvm-project/pull/98325 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix --print-libgcc-file-name on Darwin platforms (PR #98325)
@@ -0,0 +1,55 @@ +// Test the output of -print-libgcc-file-name on Darwin. + +// +// All platforms +// + +// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name \ +// RUN: --target=x86_64-apple-macos \ +// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-MACOS %s +// CHECK-CLANGRT-MACOS: libclang_rt.osx.a + +// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name \ +// RUN: --target=arm64-apple-ios \ +// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-IOS %s +// CHECK-CLANGRT-IOS: libclang_rt.ios.a + +// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name \ +// RUN: --target=arm64-apple-watchos \ +// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-WATCHOS %s +// CHECK-CLANGRT-WATCHOS: libclang_rt.watchos.a + +// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name \ +// RUN: --target=arm64-apple-tvos \ +// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-TVOS %s +// CHECK-CLANGRT-TVOS: libclang_rt.tvos.a + +// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name \ +// RUN: --target=arm64-apple-driverkit \ +// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-DRIVERKIT %s +// CHECK-CLANGRT-DRIVERKIT: libclang_rt.driverkit.a + +// TODO add simulators + +// +// Check the cc_kext variants +// + +// TODO + +// +// Check the sanitizer and profile variants +// + +// TODO + +// +// Check the dynamic library variants +// Xazax-hun wrote: That makes sense to me. Thanks! https://github.com/llvm/llvm-project/pull/98325 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix --print-libgcc-file-name on Darwin platforms (PR #98325)
@@ -0,0 +1,55 @@ +// Test the output of -print-libgcc-file-name on Darwin. + +// +// All platforms +// + +// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name \ +// RUN: --target=x86_64-apple-macos \ +// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-MACOS %s +// CHECK-CLANGRT-MACOS: libclang_rt.osx.a + +// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name \ +// RUN: --target=arm64-apple-ios \ +// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-IOS %s +// CHECK-CLANGRT-IOS: libclang_rt.ios.a + +// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name \ +// RUN: --target=arm64-apple-watchos \ +// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-WATCHOS %s +// CHECK-CLANGRT-WATCHOS: libclang_rt.watchos.a + +// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name \ +// RUN: --target=arm64-apple-tvos \ +// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-TVOS %s +// CHECK-CLANGRT-TVOS: libclang_rt.tvos.a + +// RUN: %clang -rtlib=compiler-rt -print-libgcc-file-name \ +// RUN: --target=arm64-apple-driverkit \ +// RUN: -resource-dir=%S/Inputs/resource_dir 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-CLANGRT-DRIVERKIT %s +// CHECK-CLANGRT-DRIVERKIT: libclang_rt.driverkit.a + +// TODO add simulators + +// +// Check the cc_kext variants +// + +// TODO + +// +// Check the sanitizer and profile variants +// + +// TODO Xazax-hun wrote: As per your comment below, I added tests to make sure we always print `libclang_rt..a`. https://github.com/llvm/llvm-project/pull/98325 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix --print-libgcc-file-name on Darwin platforms (PR #98325)
@@ -2271,6 +2271,12 @@ bool Driver::HandleImmediateArgs(const Compilation &C) { if (C.getArgs().hasArg(options::OPT_print_libgcc_file_name)) { ToolChain::RuntimeLibType RLT = TC.GetRuntimeLibType(C.getArgs()); const llvm::Triple Triple(TC.ComputeEffectiveClangTriple(C.getArgs())); +// The 'Darwin' toolchain is initialized only when its arguments are Xazax-hun wrote: Hmm, I only want to initialize the targets when this function returns false to avoid potential side effects when jobs are actually created or executed. This minimizes the risk of affecting existing code paths until the proper initialization is figured out in the driver. I'll look into something. https://github.com/llvm/llvm-project/pull/98325 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix --print-libgcc-file-name on Darwin platforms (PR #98325)
@@ -223,6 +223,11 @@ class LLVM_LIBRARY_VISIBILITY MachO : public ToolChain { // There aren't any profiling libs for embedded targets currently. } + // Return the full path of the compiler-rt library on a Darwin MachO system. + // Those are under /lib/darwin/<...>(.dylib|.a). + std::string getCompilerRT(const llvm::opt::ArgList &Args, StringRef Component, Xazax-hun wrote: Whoops, this was a copy and paste error inadvertently repeating the same comment twice. Now it should have the correct path in the comment. Do you think we should explain other differences here (apart from the path where the libraries can be found)? If that is the case, could you point me to some resources about non-Darwin MachO targets? https://github.com/llvm/llvm-project/pull/98325 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix --print-libgcc-file-name on Darwin platforms (PR #98325)
https://github.com/Xazax-hun updated https://github.com/llvm/llvm-project/pull/98325 From 06d63fe5be15afddd19e64bdf59ca560bc08b3b7 Mon Sep 17 00:00:00 2001 From: Gabor Horvath Date: Mon, 8 Jul 2024 11:18:02 +0100 Subject: [PATCH] Fix --print-libgcc-file-name on Darwin platforms On Darwin, --print-libgcc-file-name was returning a nonsensical result. It would return the name of the library that would be used by the default toolchain implementation, but that was something that didn't exist on Darwin. Fixing this requires initializing the Darwin toolchain before processing `--print-libgcc-file-name`. Previously, the Darwin toolchain would only be initialized when building the jobs for this compilation, which is too late since `--print-libgcc-file-name` requires the toolchain to be initialized in order to provide the right results. rdar://90633749 --- clang/include/clang/Driver/Driver.h | 5 +- clang/lib/Driver/Driver.cpp | 13 ++- clang/lib/Driver/ToolChains/Darwin.cpp| 64 clang/lib/Driver/ToolChains/Darwin.h | 13 +++ .../Driver/darwin-print-libgcc-file-name.c| 97 +++ 5 files changed, 168 insertions(+), 24 deletions(-) create mode 100644 clang/test/Driver/darwin-print-libgcc-file-name.c diff --git a/clang/include/clang/Driver/Driver.h b/clang/include/clang/Driver/Driver.h index cc1538372d5f8..04b46782467d6 100644 --- a/clang/include/clang/Driver/Driver.h +++ b/clang/include/clang/Driver/Driver.h @@ -628,8 +628,9 @@ class Driver { /// treated before building actions or binding tools. /// /// \return Whether any compilation should be built for this - /// invocation. - bool HandleImmediateArgs(const Compilation &C); + /// invocation. The compilation can only be modified when + /// this function returns false. + bool HandleImmediateArgs(Compilation &C); /// ConstructAction - Construct the appropriate action to do for /// \p Phase on the \p Input, taking in to account arguments diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index 221e222bdd47d..083756dfb62f2 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -2128,7 +2128,7 @@ void Driver::HandleAutocompletions(StringRef PassedFlags) const { llvm::outs() << llvm::join(SuggestedCompletions, "\n") << '\n'; } -bool Driver::HandleImmediateArgs(const Compilation &C) { +bool Driver::HandleImmediateArgs(Compilation &C) { // The order these options are handled in gcc is all over the place, but we // don't expect inconsistencies w.r.t. that to matter in practice. @@ -2180,6 +2180,15 @@ bool Driver::HandleImmediateArgs(const Compilation &C) { } const ToolChain &TC = C.getDefaultToolChain(); + auto initDarwinTarget = [&]() { +// The 'Darwin' toolchain is initialized only when its arguments are +// computed. Get the default arguments for OFK_None to ensure that +// initialization is performed before trying to access properties of +// the toolchain in the functions below. +// FIXME: Remove when darwin's toolchain is initialized during construction. +const llvm::Triple Triple(TC.ComputeEffectiveClangTriple(C.getArgs())); +C.getArgsForToolChain(&TC, Triple.getArchName(), Action::OFK_None); + }; if (C.getArgs().hasArg(options::OPT_v)) TC.printVerboseInfo(llvm::errs()); @@ -2230,6 +2239,7 @@ bool Driver::HandleImmediateArgs(const Compilation &C) { } if (C.getArgs().hasArg(options::OPT_print_runtime_dir)) { +initDarwinTarget(); if (std::optional RuntimePath = TC.getRuntimePath()) llvm::outs() << *RuntimePath << '\n'; else @@ -2271,6 +2281,7 @@ bool Driver::HandleImmediateArgs(const Compilation &C) { if (C.getArgs().hasArg(options::OPT_print_libgcc_file_name)) { ToolChain::RuntimeLibType RLT = TC.GetRuntimeLibType(C.getArgs()); const llvm::Triple Triple(TC.ComputeEffectiveClangTriple(C.getArgs())); +initDarwinTarget(); RegisterEffectiveTriple TripleRAII(TC, Triple); switch (RLT) { case ToolChain::RLT_CompilerRT: diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index f354b0974d5f2..e530e45a9793b 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -1272,23 +1272,8 @@ unsigned DarwinClang::GetDefaultDwarfVersion() const { void MachO::AddLinkRuntimeLib(const ArgList &Args, ArgStringList &CmdArgs, StringRef Component, RuntimeLinkOptions Opts, bool IsShared) const { - SmallString<64> DarwinLibName = StringRef("libclang_rt."); - // On Darwin the builtins component is not in the library name. - if (Component != "builtins") { -DarwinLibName += Component; -if (!(Opts & RLO_IsEmbedded)) - DarwinLibName += "_"; - } - - DarwinLibName += getOSLibraryNameSuffix(); - DarwinLibName += IsShared ? "_dynamic.dylib" : ".a"; - SmallString<128> Dir
[clang] Fix --print-libgcc-file-name on Darwin platforms (PR #98325)
https://github.com/Xazax-hun created https://github.com/llvm/llvm-project/pull/98325 On Darwin, --print-libgcc-file-name was returning a nonsensical result. It would return the name of the library that would be used by the default toolchain implementation, but that was something that didn't exist on Darwin. Fixing this requires initializing the Darwin toolchain before processing `--print-libgcc-file-name`. Previously, the Darwin toolchain would only be initialized when building the jobs for this compilation, which is too late since `--print-libgcc-file-name` requires the toolchain to be initialized in order to provide the right results. From bbd66df28f451b79dccf6c879e355e2f555e48cd Mon Sep 17 00:00:00 2001 From: Gabor Horvath Date: Mon, 8 Jul 2024 11:18:02 +0100 Subject: [PATCH] Fix --print-libgcc-file-name on Darwin platforms On Darwin, --print-libgcc-file-name was returning a nonsensical result. It would return the name of the library that would be used by the default toolchain implementation, but that was something that didn't exist on Darwin. Fixing this requires initializing the Darwin toolchain before processing `--print-libgcc-file-name`. Previously, the Darwin toolchain would only be initialized when building the jobs for this compilation, which is too late since `--print-libgcc-file-name` requires the toolchain to be initialized in order to provide the right results. --- clang/include/clang/Driver/Driver.h | 5 +- clang/lib/Driver/Driver.cpp | 8 ++- clang/lib/Driver/ToolChains/Darwin.cpp| 61 +-- clang/lib/Driver/ToolChains/Darwin.h | 9 +++ .../Driver/darwin-print-libgcc-file-name.c| 55 + 5 files changed, 115 insertions(+), 23 deletions(-) create mode 100644 clang/test/Driver/darwin-print-libgcc-file-name.c diff --git a/clang/include/clang/Driver/Driver.h b/clang/include/clang/Driver/Driver.h index cc1538372d5f8..04b46782467d6 100644 --- a/clang/include/clang/Driver/Driver.h +++ b/clang/include/clang/Driver/Driver.h @@ -628,8 +628,9 @@ class Driver { /// treated before building actions or binding tools. /// /// \return Whether any compilation should be built for this - /// invocation. - bool HandleImmediateArgs(const Compilation &C); + /// invocation. The compilation can only be modified when + /// this function returns false. + bool HandleImmediateArgs(Compilation &C); /// ConstructAction - Construct the appropriate action to do for /// \p Phase on the \p Input, taking in to account arguments diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index 221e222bdd47d..cb1bc7e36d39d 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -2128,7 +2128,7 @@ void Driver::HandleAutocompletions(StringRef PassedFlags) const { llvm::outs() << llvm::join(SuggestedCompletions, "\n") << '\n'; } -bool Driver::HandleImmediateArgs(const Compilation &C) { +bool Driver::HandleImmediateArgs(Compilation &C) { // The order these options are handled in gcc is all over the place, but we // don't expect inconsistencies w.r.t. that to matter in practice. @@ -2271,6 +2271,12 @@ bool Driver::HandleImmediateArgs(const Compilation &C) { if (C.getArgs().hasArg(options::OPT_print_libgcc_file_name)) { ToolChain::RuntimeLibType RLT = TC.GetRuntimeLibType(C.getArgs()); const llvm::Triple Triple(TC.ComputeEffectiveClangTriple(C.getArgs())); +// The 'Darwin' toolchain is initialized only when its arguments are +// computed. Get the default arguments for OFK_None to ensure that +// initialization is performed before trying to access properties of +// the toolchain in the functions below. +// FIXME: Remove when darwin's toolchain is initialized during construction. +C.getArgsForToolChain(&TC, Triple.getArchName(), Action::OFK_None); RegisterEffectiveTriple TripleRAII(TC, Triple); switch (RLT) { case ToolChain::RLT_CompilerRT: diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp index f354b0974d5f2..c2e90a5c0e49c 100644 --- a/clang/lib/Driver/ToolChains/Darwin.cpp +++ b/clang/lib/Driver/ToolChains/Darwin.cpp @@ -1272,23 +1272,8 @@ unsigned DarwinClang::GetDefaultDwarfVersion() const { void MachO::AddLinkRuntimeLib(const ArgList &Args, ArgStringList &CmdArgs, StringRef Component, RuntimeLinkOptions Opts, bool IsShared) const { - SmallString<64> DarwinLibName = StringRef("libclang_rt."); - // On Darwin the builtins component is not in the library name. - if (Component != "builtins") { -DarwinLibName += Component; -if (!(Opts & RLO_IsEmbedded)) - DarwinLibName += "_"; - } - - DarwinLibName += getOSLibraryNameSuffix(); - DarwinLibName += IsShared ? "_dynamic.dylib" : ".a"; - SmallString<128> Dir(getDriver().ResourceDir); - llvm::sys::path::append(Dir, "lib", "darwin"); - if (Opts & RL
[clang] [analyzer][docs] Add clang-19 release notes for CSA (PR #97418)
Xazax-hun wrote: Small note, the screenshot has the "improvements" part, but I did not find that in the actual PR. https://github.com/llvm/llvm-project/pull/97418 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer][docs] Add clang-19 release notes for CSA (PR #97418)
https://github.com/Xazax-hun approved this pull request. LGTM! https://github.com/llvm/llvm-project/pull/97418 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Remove a pointer union from the lifetime bound analysis (PR #97327)
https://github.com/Xazax-hun closed https://github.com/llvm/llvm-project/pull/97327 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Remove a pointer union from the lifetime bound analysis (PR #97327)
https://github.com/Xazax-hun created https://github.com/llvm/llvm-project/pull/97327 Since all callers of the API is called with the same type, we do not actually need the pointer union. From 48f51032cb9228ea8704e4e2352b18ea182e1024 Mon Sep 17 00:00:00 2001 From: Gabor Horvath Date: Mon, 1 Jul 2024 18:29:34 +0100 Subject: [PATCH] [clang] Remove a pointer union from the lifetime bound analysis Since all callers of the API is called with the same type, we do not actually need the pointer union. --- clang/lib/Sema/CheckExprLifetime.cpp | 36 ++-- 1 file changed, 13 insertions(+), 23 deletions(-) diff --git a/clang/lib/Sema/CheckExprLifetime.cpp b/clang/lib/Sema/CheckExprLifetime.cpp index be77949e8b547..fea0239f7bc36 100644 --- a/clang/lib/Sema/CheckExprLifetime.cpp +++ b/clang/lib/Sema/CheckExprLifetime.cpp @@ -966,27 +966,11 @@ static bool pathOnlyInitializesGslPointer(IndirectLocalPath &Path) { return false; } -static void checkExprLifetimeImpl( -Sema &SemaRef, -llvm::PointerUnion -CEntity, -Expr *Init) { - LifetimeKind LK = LK_FullExpression; - - const AssignedEntity *AEntity = nullptr; - // Local variables for initialized entity. - const InitializedEntity *InitEntity = nullptr; - const InitializedEntity *ExtendingEntity = nullptr; - if (CEntity.is()) { -InitEntity = CEntity.get(); -auto LTResult = getEntityLifetime(InitEntity); -LK = LTResult.getInt(); -ExtendingEntity = LTResult.getPointer(); - } else { -AEntity = CEntity.get(); -if (AEntity->LHS->getType()->isPointerType()) // builtin pointer type - LK = LK_Extended; - } +static void checkExprLifetimeImpl(Sema &SemaRef, + const InitializedEntity *InitEntity, + const InitializedEntity *ExtendingEntity, + LifetimeKind LK, + const AssignedEntity *AEntity, Expr *Init) { // If this entity doesn't have an interesting lifetime, don't bother looking // for temporaries within its initializer. if (LK == LK_FullExpression) @@ -1292,12 +1276,18 @@ static void checkExprLifetimeImpl( void checkExprLifetime(Sema &SemaRef, const InitializedEntity &Entity, Expr *Init) { - checkExprLifetimeImpl(SemaRef, &Entity, Init); + auto LTResult = getEntityLifetime(&Entity); + LifetimeKind LK = LTResult.getInt(); + const InitializedEntity *ExtendingEntity = LTResult.getPointer(); + checkExprLifetimeImpl(SemaRef, &Entity, ExtendingEntity, LK, nullptr, Init); } void checkExprLifetime(Sema &SemaRef, const AssignedEntity &Entity, Expr *Init) { - checkExprLifetimeImpl(SemaRef, &Entity, Init); + LifetimeKind LK = LK_FullExpression; + if (Entity.LHS->getType()->isPointerType()) // builtin pointer type +LK = LK_Extended; + checkExprLifetimeImpl(SemaRef, nullptr, nullptr, LK, &Entity, Init); } } // namespace clang::sema ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Extend lifetime bound analysis to support assignments (PR #96475)
https://github.com/Xazax-hun approved this pull request. https://github.com/llvm/llvm-project/pull/96475 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Extend lifetime bound analysis to support assignments (PR #96475)
@@ -964,17 +966,34 @@ static bool pathOnlyInitializesGslPointer(IndirectLocalPath &Path) { return false; } -void checkExprLifetime(Sema &SemaRef, const InitializedEntity &Entity, +void checkExprLifetime(Sema &SemaRef, const CheckingEntity &CEntity, Xazax-hun wrote: This! This already looks much better! I think we can get rid of the pointer union as well by sort of "inlining" the two branches of the if statement in the implementation function into the corresponding overloads. But I am also OK with the current PR as is. https://github.com/llvm/llvm-project/pull/96475 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Extend lifetime bound analysis to support assignments (PR #96475)
https://github.com/Xazax-hun edited https://github.com/llvm/llvm-project/pull/96475 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Extend lifetime bound analysis to support assignments (PR #96475)
@@ -964,17 +966,34 @@ static bool pathOnlyInitializesGslPointer(IndirectLocalPath &Path) { return false; } -void checkExprLifetime(Sema &SemaRef, const InitializedEntity &Entity, +void checkExprLifetime(Sema &SemaRef, const CheckingEntity &CEntity, Xazax-hun wrote: I am sort of wondering of we need a `variant` at all for this API. We always know statically whether we call with an InitializedEntity or an AssignedEntity. So I wonder if it would make more sense to have an overload set, both of which would call into something like `checkExprLifetimeImpl(LifetimeKind, AssignedEntity, InitializedEntity, InitializedEntity). https://github.com/llvm/llvm-project/pull/96475 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Extend lifetime bound analysis to support assignments (PR #96475)
https://github.com/Xazax-hun approved this pull request. LGTM, with a nit. https://github.com/llvm/llvm-project/pull/96475 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][docs] Update the gsl::Pointer/Owner attr doc (PR #96908)
https://github.com/Xazax-hun approved this pull request. https://github.com/llvm/llvm-project/pull/96908 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Extend lifetime bound analysis to support assignments (PR #96475)
https://github.com/Xazax-hun edited https://github.com/llvm/llvm-project/pull/96475 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Extend lifetime bound analysis to support assignments (PR #96475)
@@ -964,11 +966,26 @@ static bool pathOnlyInitializesGslPointer(IndirectLocalPath &Path) { return false; } -void checkExprLifetime(Sema &SemaRef, const InitializedEntity &Entity, +void checkExprLifetime(Sema &SemaRef, const CheckingEntity &CEntity, Expr *Init) { - LifetimeResult LR = getEntityLifetime(&Entity); - LifetimeKind LK = LR.getInt(); - const InitializedEntity *ExtendingEntity = LR.getPointer(); + LifetimeKind LK = LK_FullExpression; + + const AssignedEntity *AEntity = nullptr; + // Local variables for initialized entity. + const InitializedEntity *InitEntity = nullptr; + const InitializedEntity *ExtendingEntity = nullptr; + if (auto IEntityP = std::get_if(&CEntity)) { +InitEntity = *IEntityP; +auto LTResult = getEntityLifetime(InitEntity); +LK = LTResult.getInt(); +ExtendingEntity = LTResult.getPointer(); + } else if (auto AEntityP = std::get_if(&CEntity)) { +AEntity = *AEntityP; +if (AEntity->LHS->getType()->isPointerType()) // builtin pointer type + LK = LK_Extended; Xazax-hun wrote: I am a bit confused here, could you elaborate why we want `LK_Extended` here? As fas as I remember, assignments are not doing lifetime extension. https://github.com/llvm/llvm-project/pull/96475 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer][NFC] Use ArrayRef for input parameters (PR #93203)
@@ -672,7 +672,7 @@ class StdLibraryFunctionsChecker StringRef getNote() const { return Note; } }; - using ArgTypes = std::vector>; + using ArgTypes = ArrayRef>; Xazax-hun wrote: I am also wondering if `optional` is a smell, since QualType can be null. https://github.com/llvm/llvm-project/pull/93203 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer][NFC] Use ArrayRef for input parameters (PR #93203)
https://github.com/Xazax-hun approved this pull request. https://github.com/llvm/llvm-project/pull/93203 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Move the initializer lifetime checking code from SemaInit.cpp to a new place, NFC (PR #96758)
@@ -0,0 +1,1258 @@ +//===--- CheckExprLifetime.cpp ===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "CheckExprLifetime.h" +#include "clang/AST/Expr.h" +#include "clang/Sema/Sema.h" +#include "llvm/ADT/PointerIntPair.h" + +namespace clang::sema { +namespace { +enum LifetimeKind { + /// The lifetime of a temporary bound to this entity ends at the end of the + /// full-expression, and that's (probably) fine. + LK_FullExpression, + + /// The lifetime of a temporary bound to this entity is extended to the + /// lifeitme of the entity itself. + LK_Extended, + + /// The lifetime of a temporary bound to this entity probably ends too soon, + /// because the entity is allocated in a new-expression. + LK_New, + + /// The lifetime of a temporary bound to this entity ends too soon, because + /// the entity is a return object. + LK_Return, + + /// The lifetime of a temporary bound to this entity ends too soon, because + /// the entity is the result of a statement expression. + LK_StmtExprResult, + + /// This is a mem-initializer: if it would extend a temporary (other than via + /// a default member initializer), the program is ill-formed. + LK_MemInitializer, +}; +using LifetimeResult = +llvm::PointerIntPair; +} Xazax-hun wrote: Nit: we have many static functions here. Maybe we could just have one big anonymous namespace for most of this file and remove the `static` keywords? https://github.com/llvm/llvm-project/pull/96758 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Move the initializer lifetime checking code from SemaInit.cpp to a new place, NFC (PR #96758)
@@ -0,0 +1,1258 @@ +//===--- CheckExprLifetime.cpp ===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "CheckExprLifetime.h" +#include "clang/AST/Expr.h" +#include "clang/Sema/Sema.h" +#include "llvm/ADT/PointerIntPair.h" + +namespace clang::sema { +namespace { +enum LifetimeKind { + /// The lifetime of a temporary bound to this entity ends at the end of the + /// full-expression, and that's (probably) fine. + LK_FullExpression, + + /// The lifetime of a temporary bound to this entity is extended to the + /// lifeitme of the entity itself. + LK_Extended, + + /// The lifetime of a temporary bound to this entity probably ends too soon, + /// because the entity is allocated in a new-expression. + LK_New, + + /// The lifetime of a temporary bound to this entity ends too soon, because + /// the entity is a return object. + LK_Return, + + /// The lifetime of a temporary bound to this entity ends too soon, because + /// the entity is the result of a statement expression. + LK_StmtExprResult, + + /// This is a mem-initializer: if it would extend a temporary (other than via + /// a default member initializer), the program is ill-formed. + LK_MemInitializer, +}; +using LifetimeResult = +llvm::PointerIntPair; +} Xazax-hun wrote: I am also open to do these changes in a followup PR, as not changing moved code might be cleaner for the history. https://github.com/llvm/llvm-project/pull/96758 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Move the initializer lifetime checking code from SemaInit.cpp to a new place, NFC (PR #96758)
https://github.com/Xazax-hun approved this pull request. https://github.com/llvm/llvm-project/pull/96758 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Move the initializer lifetime checking code from SemaInit.cpp to a new place, NFC (PR #96758)
https://github.com/Xazax-hun edited https://github.com/llvm/llvm-project/pull/96758 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Teach `AnalysisASTVisitor` that `typeid()` can be evaluated. (PR #96731)
https://github.com/Xazax-hun approved this pull request. https://github.com/llvm/llvm-project/pull/96731 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][nullability] Improve modeling of `++`/`--` operators. (PR #96601)
https://github.com/Xazax-hun approved this pull request. https://github.com/llvm/llvm-project/pull/96601 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Extend lifetime bound analysis to support assignments (PR #96475)
Xazax-hun wrote: +1, I like Ilya's suggestion. https://github.com/llvm/llvm-project/pull/96475 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] Extend lifetime bound analysis to support assignments (PR #96475)
Xazax-hun wrote: Wow, this is awesome! Thanks for tackling this! https://github.com/llvm/llvm-project/pull/96475 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Add a callback run on the pre-transfer state. (PR #96140)
https://github.com/Xazax-hun approved this pull request. https://github.com/llvm/llvm-project/pull/96140 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits