Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[[clang::lifetimebound]] is ignored without warning if the function parameter doesn't have a name. #96034

Open
hokein opened this issue Jun 19, 2024 · 5 comments
Labels
clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:memory-safety Issue/FR relating to the lifetime analysis in Clang (-Wdangling, -Wreturn-local-addr)

Comments

@hokein
Copy link
Collaborator

hokein commented Jun 19, 2024

See the example:

int* good(const int& abc[[clang::lifetimebound]]);

int* bad(const int& [[clang::lifetimebound]]);

The AST looks like:

-FunctionDecl  ... good 'int *(const int &)'
| `-ParmVarDecl ... abc 'const int &'
|   `-LifetimeBoundAttr 
`-FunctionDecl ... bad 'int *(const int &)'
  `-ParmVarDecl ... 'const int &'
@hokein hokein added the clang:frontend Language frontend issues, e.g. anything involving "Sema" label Jun 19, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 19, 2024

@llvm/issue-subscribers-clang-frontend

Author: Haojian Wu (hokein)

See the example:

int* good(const int& abc[[clang::lifetimebound]]);

int* bad(const int& [[clang::lifetimebound]]);

The AST looks like:

-FunctionDecl  ... good 'int *(const int &)'
| `-ParmVarDecl ... abc 'const int &'
|   `-LifetimeBoundAttr 
`-FunctionDecl ... bad 'int *(const int &)'
  `-ParmVarDecl ... 'const int &'

@MitalAshok
Copy link
Contributor

MitalAshok commented Jun 19, 2024

It looks like the attribute is just ignored without warning at that position: https://godbolt.org/z/nM56PbTbe

int* good(const int& abc[[clang::lifetimebound]]);
int* bad(const int& [[clang::lifetimebound]]);
int* good2([[clang::lifetimebound]] const int&);
int* good3([[clang::lifetimebound]] const int& abc);
int* bad2(const int& [[clang::lifetimebound]] abc);

int main() {
    int* a = good(0);
    int* b = bad(0); // no -Wdangling
    int* c = good2(0);
    int* d = good3(0);
    int* e = bad2(0); // no -Wdangling
}

The fact that it doesn't work makes sense as [[clang::lifetimebound]] is supposed to apply to the parameter, not the type of the parameter, which is what it would do at that position. It looks like it is supposed to error for bad and bad2, but just doesn't.

Compare that to the similar [[gnu::nonnull]]: https://godbolt.org/z/1jKrEq7MG

int* good(const int* abc[[gnu::nonnull]]);
int* bad(const int* [[gnu::nonnull]]);  // warning: attribute ignored
int* good2([[gnu::nonnull]] const int*);
int* good3([[gnu::nonnull]] const int* abc);
int* bad2(const int* [[gnu::nonnull]] abc);  // warning: attribute ignored

int main() {
    int* a = good(0);
    int* b = bad(0); // no warning
    int* c = good2(0);
    int* d = good3(0);
    int* e = bad2(0); // no warning
}

let Subjects = SubjectList<[ParmVar, ImplicitObjectParameter], ErrorDiag>;

let Subjects = SubjectList<[ObjCMethod, HasFunctionProto, ParmVar], WarnDiag,

@hokein
Copy link
Collaborator Author

hokein commented Jun 21, 2024

Thanks for the clarification.

The fact that it doesn't work makes sense as [[clang::lifetimebound]] is supposed to apply to the parameter, not the type of the parameter, which is what it would do at that position. It looks like it is supposed to error for bad and bad2, but just doesn't.

Yeah, this makes sense. For the bad case, it would be great if Clang emitted an attribute-ignored warning. I was confused by this when experimenting with the lifetimebound feature.

@usx95 usx95 changed the title [[clang::lifetimebound]] is missing if the function parameter doesn't have a name. [[clang::lifetimebound]] is ignored without warning if the function parameter doesn't have a name. Jul 24, 2024
@usx95 usx95 added the clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer label Jul 24, 2024
@zygoloid
Copy link
Collaborator

I think we're not diagnosing this as a non-type attribute applied to a type because it is a type attribute -- specifically, when applied to ah implicit object parameter, what it actually appertains to is a function type. We'll need some better checking for the ImplicitObjectParameter subject that diagnoses uses as a type attribute in a position other than on the (innermost) function declarator of a member function.

@emaxx-google
Copy link
Contributor

I've published #118281 which adds warnings to diagnose this and similar cases of a silent [[clang::lifetimebound]] ignore.

hokein pushed a commit that referenced this issue Dec 3, 2024
Emit the "cannot be applied to types" warning instead of silently
ignoring the attribute when it's attempted to be used on a type (instead
of a function argument or the function definition).

Before this commit, the warning has been printed when the attribute was
(mis)used on a decl-specifier, but not in other places in a declarator.

Examples where the warning starts being emitted with this commit:

```
  int * [[clang::lifetimebound]] x;

  void f(int * [[clang::lifetimebound]] x);

  void g(int * [[clang::lifetimebound]]);
```

Note that the last example is the case of an unnamed function parameter.
While in theory Clang could've supported the `[[clang::lifetimebound]]`
analysis for unnamed parameters, it doesn't currently, so the commit at
least makes the situation better by highlighting this as a warning
instead of a silent ignore - which was reported at #96034.
hokein pushed a commit that referenced this issue Jan 10, 2025
…18501)

This relands #118281 as-is, after it got reverted in commit
356df2d. The reland can go in after we
fixed some downstream codebases that had incorrectly placed attributes.

Original commit description:

> Emit the "cannot be applied to types" warning instead of silently
ignoring the attribute when it's attempted to be used on a type (instead
of a function argument or the function definition).
>
> Before this commit, the warning has been printed when the attribute
was (mis)used on a decl-specifier, but not in other places in a
declarator.
>
> Examples where the warning starts being emitted with this commit:
>
> ```
>   int * [[clang::lifetimebound]] x;
>
>   void f(int * [[clang::lifetimebound]] x);
>
>   void g(int * [[clang::lifetimebound]]);
> ```
>
> Note that the last example is the case of an unnamed function
parameter. While in theory Clang could've supported the
`[[clang::lifetimebound]]` analysis for unnamed parameters, it doesn't
currently, so the commit at least makes the situation better by
highlighting this as a warning instead of a silent ignore - which was
reported at #96034.
BaiXilin pushed a commit to BaiXilin/llvm-fix-vnni-instr-types that referenced this issue Jan 12, 2025
…vm#118501)

This relands llvm#118281 as-is, after it got reverted in commit
356df2d. The reland can go in after we
fixed some downstream codebases that had incorrectly placed attributes.

Original commit description:

> Emit the "cannot be applied to types" warning instead of silently
ignoring the attribute when it's attempted to be used on a type (instead
of a function argument or the function definition).
>
> Before this commit, the warning has been printed when the attribute
was (mis)used on a decl-specifier, but not in other places in a
declarator.
>
> Examples where the warning starts being emitted with this commit:
>
> ```
>   int * [[clang::lifetimebound]] x;
>
>   void f(int * [[clang::lifetimebound]] x);
>
>   void g(int * [[clang::lifetimebound]]);
> ```
>
> Note that the last example is the case of an unnamed function
parameter. While in theory Clang could've supported the
`[[clang::lifetimebound]]` analysis for unnamed parameters, it doesn't
currently, so the commit at least makes the situation better by
highlighting this as a warning instead of a silent ignore - which was
reported at llvm#96034.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:memory-safety Issue/FR relating to the lifetime analysis in Clang (-Wdangling, -Wreturn-local-addr)
Projects
None yet
Development

No branches or pull requests

6 participants