https://github.com/AaronBallman closed
https://github.com/llvm/llvm-project/pull/80040
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/Endilll commented:
`Sema.h` changes look good to me.
https://github.com/llvm/llvm-project/pull/80040
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
11happy wrote:
> It looks like it passed on your last commit but you have a conflict now which
> you need to resolve.
>
> Can you merge or do you need help with that?
I tried to resolve the merge conflict but the resolve editor is just stuck on
loading on Vs Code may be as it have 2
shafik wrote:
It looks like it passed on your last commit but you have a conflict now which
you need to resolve.
Can you merge or do you need help with that?
https://github.com/llvm/llvm-project/pull/80040
___
cfe-commits mailing list
https://github.com/shafik commented:
The failure of the Windows CI is due to a know problem w/ false detection of a
virus. You can use `--allow-empty` to make an empty commit and restart the
process. You have approval, so once it goes green you should be good to squash
and merge.
erichkeane wrote:
> what would be the reason for windows build failing , is it a CI issue or
> specific to this PR & what can I do to resolve that. Thank you
That appears to be a problem with the CI itself. I think we've fixed up a
bunch of the CI, but it will require doing a 'merge' with
11happy wrote:
what would be the reason for windows build failing , is it a CI issue or
specific to this PR & what can I do to resolve that.
Thank you
https://github.com/llvm/llvm-project/pull/80040
___
cfe-commits mailing list
11happy wrote:
Sorry, I left the previous function declaration in the `sema.h`, I have removed
it now.
https://github.com/llvm/llvm-project/pull/80040
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://github.com/AaronBallman approved this pull request.
LGTM!
https://github.com/llvm/llvm-project/pull/80040
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/erichkeane approved this pull request.
https://github.com/llvm/llvm-project/pull/80040
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/AaronBallman commented:
Precommit CI found a valid failure on Windows:
```
FAIL: Clang :: Sema/attr-cleanup.c (15237 of 19292)
TEST 'Clang :: Sema/attr-cleanup.c' FAILED
Exit Code: 1
Command Output (stdout):
--
# RUN: at line 1
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 %s -verify -fsyntax-only
+// RUN: %clang_cc1 -Wfree-nonheap-object -fsyntax-only -verify %s
void c1(int *a);
-
+typedef unsigned long size_t;
AaronBallman wrote:
```suggestion
typedef __typeof__(sizeof(0)) size_t;
```
https://github.com/AaronBallman edited
https://github.com/llvm/llvm-project/pull/80040
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
11happy wrote:
sorry for last 3 continuous format code commits, I usually run `git
clang-format HEAD~1` and also ran this time idk why it was not correctly
formatted so I tried manually and it resulted in so many commits,
Thank you
https://github.com/llvm/llvm-project/pull/80040
11happy wrote:
I have added the tests , and its working as expected . Clang-diagnostics is
indeed interesting to work on.
Thank you
https://github.com/llvm/llvm-project/pull/80040
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
@@ -3780,6 +3780,30 @@ static void handleCleanupAttr(Sema , Decl *D, const
ParsedAttr ) {
<< NI.getName() << ParamTy << Ty;
return;
}
+ VarDecl *VD = cast(D);
+ // Create a reference to the variable declaration. This is a fake/dummy
+ // reference.
+
@@ -3780,6 +3780,30 @@ static void handleCleanupAttr(Sema , Decl *D, const
ParsedAttr ) {
<< NI.getName() << ParamTy << Ty;
return;
}
+ VarDecl *VD = cast(D);
+ // Create a reference to the variable declaration. This is a fake/dummy
+ // reference.
+
@@ -3780,6 +3780,30 @@ static void handleCleanupAttr(Sema , Decl *D, const
ParsedAttr ) {
<< NI.getName() << ParamTy << Ty;
return;
}
+ VarDecl *VD = cast(D);
+ // Create a reference to the variable declaration. This is a fake/dummy
+ // reference.
+
@@ -3780,6 +3780,30 @@ static void handleCleanupAttr(Sema , Decl *D, const
ParsedAttr ) {
<< NI.getName() << ParamTy << Ty;
return;
}
+ VarDecl *VD = cast(D);
+ // Create a reference to the variable declaration. This is a fake/dummy
+ // reference.
+
11happy wrote:
Sorry I was not able to look into the comments for last few days as semester
exams are currently going on, I have added the commit suggestions currently and
will update with the tests I have written by sunday as exams end.
Thank you.
@@ -3780,6 +3780,30 @@ static void handleCleanupAttr(Sema , Decl *D, const
ParsedAttr ) {
<< NI.getName() << ParamTy << Ty;
return;
}
+ VarDecl *VD = cast(D);
+ // Create a reference to the variable declaration. This is a fake/dummy
+ // reference.
+
@@ -3780,6 +3780,30 @@ static void handleCleanupAttr(Sema , Decl *D, const
ParsedAttr ) {
<< NI.getName() << ParamTy << Ty;
return;
}
+ VarDecl *VD = cast(D);
+ // Create a reference to the variable declaration. This is a fake/dummy
+ // reference.
+
@@ -3780,6 +3780,30 @@ static void handleCleanupAttr(Sema , Decl *D, const
ParsedAttr ) {
<< NI.getName() << ParamTy << Ty;
return;
}
+ VarDecl *VD = cast(D);
+ // Create a reference to the variable declaration. This is a fake/dummy
+ // reference.
+
@@ -3780,6 +3780,30 @@ static void handleCleanupAttr(Sema , Decl *D, const
ParsedAttr ) {
<< NI.getName() << ParamTy << Ty;
return;
}
+ VarDecl *VD = cast(D);
+ // Create a reference to the variable declaration. This is a fake/dummy
+ // reference.
+
11happy wrote:
I have fixed the merge conflict , will be adding more tests as I am able to
test them locally.
Thank you
https://github.com/llvm/llvm-project/pull/80040
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
11happy wrote:
I have added a test, but will be adding more in some upcoming commits as I test
them locally.
https://github.com/llvm/llvm-project/pull/80040
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
11happy wrote:
Got it
https://github.com/llvm/llvm-project/pull/80040
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
erichkeane wrote:
> Umm how could I resolve that? I tried various things but none working. 沈
You'll have to spend some time in a debugger (note that
`clang::DiagnosticIDs::EmitDiag` is where the diagnostic happens, so is a good
place for a brekapoint). But I see that in your creation of your
11happy wrote:
Umm how could I resolve that? I tried various things but none working. 沈
https://github.com/llvm/llvm-project/pull/80040
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
erichkeane wrote:
> ```
> extern void free(void *);
> extern void *malloc(size_t size);
> void t8(void) {
> void *p __attribute__((cleanup(free))) = malloc(10); //
> expected-warning{{attempt to call free on non-heap object 'p'}}
> }
> ```
>
> I added this, but I am getting this error:
11happy wrote:
```
extern void free(void *);
extern void *malloc(size_t size);
void t8(void) {
void *p __attribute__((cleanup(free))) = malloc(10); //
expected-warning{{attempt to call free on non-heap object 'p'}}
}
```
I added this, but
I am getting this error: (Sorry if its a silly
11happy wrote:
> extern void free(void *);
Sure I already wrote some tests and will commit after locally checking them
but idk everytime I restart pc clang rebuilds itself and that is taking time,
is it normal for clang to rebuild everytime I restart? (I think it has
something to do with
https://github.com/AaronBallman commented:
I think the only thing left is to add test coverage somewhere in
`clang/test/Sema/`, probably in
https://github.com/llvm/llvm-project/blob/main/clang/test/Sema/attr-cleanup.c.
You can probably test with something like:
```
extern void free(void *);
@@ -106,6 +106,9 @@ Improvements to Clang's diagnostics
- Clang now applies syntax highlighting to the code snippets it
prints.
+- Clang now provides improved warnings for the cleanup attribute to detect
misuse scenarios,
+ such as attempting to call `free` on unallocated
AaronBallman wrote:
> tried to implement the requested changes along the new direction, I have some
> queries should I let this statament as is or assign to a variable:
>
> ```
> S.CheckFunctionCall(FD, FunctionCallExpression,
> FD->getType()->getAs());
> ```
That call
11happy wrote:
tried to implement the requested changes along the new direction, further I
have some queries should I let this statament as is or assign to a variable:
```
S.CheckFunctionCall(FD, FunctionCallExpression,
FD->getType()->getAs());
```
Further I request some
https://github.com/11happy updated
https://github.com/llvm/llvm-project/pull/80040
>From 93adb872d0e18ff3a1356ab47527d81b61c920cd Mon Sep 17 00:00:00 2001
From: 11happy
Date: Tue, 30 Jan 2024 23:19:04 +0530
Subject: [PATCH 1/5] Diagnose misuse of the cleanup attribute
Signed-off-by: 11happy
https://github.com/AaronBallman commented:
Thank you for working on this! I think we want to go in a different direction
for this fix. I think the real issue is that we're missing a call to
`Sema::CheckFunctionCall()` within `handleCleanupAttr()`. That's currently a
private function in
@@ -10098,6 +10098,23 @@ static bool isVector(QualType QT, QualType
ElementType) {
return false;
}
+bool Sema::IsPointerToPointer(QualType LHSType, QualType RHSType) {
+ if (const PointerType *LHSPointer = dyn_cast(LHSType)) {
erichkeane wrote:
I think
@@ -8265,6 +8265,9 @@ def warn_condition_is_assignment : Warning<"using the
result of an "
def warn_free_nonheap_object
: Warning<"attempt to call %0 on non-heap %select{object %2|object: block
expression|object: lambda-to-function-pointer conversion}1">,
InGroup;
+def
https://github.com/erichkeane edited
https://github.com/llvm/llvm-project/pull/80040
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/erichkeane commented:
This needs a release note and some tests as well.
https://github.com/llvm/llvm-project/pull/80040
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://github.com/11happy updated
https://github.com/llvm/llvm-project/pull/80040
>From 93adb872d0e18ff3a1356ab47527d81b61c920cd Mon Sep 17 00:00:00 2001
From: 11happy
Date: Tue, 30 Jan 2024 23:19:04 +0530
Subject: [PATCH 1/4] Diagnose misuse of the cleanup attribute
Signed-off-by: 11happy
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff 7c03d5d41daad230406890499cf4fa14973ee5eb
1a3cc18276be89969d5a262ff12499d20fd925ee --
11happy wrote:
I was going through the thread on discord whether we should use same
diagnostics as GCC or not, so as suggested there we should not directly copy
but having same essence will be fine. so in recent commit changed the warning.
Thank you
https://github.com/11happy updated
https://github.com/llvm/llvm-project/pull/80040
>From 93adb872d0e18ff3a1356ab47527d81b61c920cd Mon Sep 17 00:00:00 2001
From: 11happy
Date: Tue, 30 Jan 2024 23:19:04 +0530
Subject: [PATCH 1/3] Diagnose misuse of the cleanup attribute
Signed-off-by: 11happy
https://github.com/11happy updated
https://github.com/llvm/llvm-project/pull/80040
>From 93adb872d0e18ff3a1356ab47527d81b61c920cd Mon Sep 17 00:00:00 2001
From: 11happy
Date: Tue, 30 Jan 2024 23:19:04 +0530
Subject: [PATCH 1/2] Diagnose misuse of the cleanup attribute
Signed-off-by: 11happy
11happy wrote:
the current working is as expected as per my understanding, However I request
comments & suggestions from others as I am not quite sure that this would be
fine,also apologies for not so good naming.
Thank you
https://github.com/llvm/llvm-project/pull/80040
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Bhuminjay Soni (11happy)
Changes
**Overview:**
This pull request fixes #79443 when the cleanup attribute is intended
to be applied to a variable declaration, passing its address to a specified
function. The problem arises when standard
https://github.com/11happy created
https://github.com/llvm/llvm-project/pull/80040
**Overview:**
This pull request fixes #79443 when the cleanup attribute is intended to be
applied to a variable declaration, passing its address to a specified function.
The problem arises when standard
50 matches
Mail list logo
