github-actions[bot] wrote:
@T-Gruber Congratulations on having your first Pull Request (PR) merged into
the LLVM Project!
Your changes will be combined with recent changes from other authors, then
tested
by our [build bots](https://lab.llvm.org/buildbot/). If there is a problem with
a
https://github.com/steakhal closed
https://github.com/llvm/llvm-project/pull/85104
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/steakhal updated
https://github.com/llvm/llvm-project/pull/85104
>From 0f964127ed91e23f8e969e08ce680535cfeb8906 Mon Sep 17 00:00:00 2001
From: Andreas Steinhausen
Date: Wed, 13 Mar 2024 17:07:53 +0100
Subject: [PATCH 01/12] Adapted MemRegion::getDescriptiveName to handle
https://github.com/T-Gruber updated
https://github.com/llvm/llvm-project/pull/85104
>From 0f964127ed91e23f8e969e08ce680535cfeb8906 Mon Sep 17 00:00:00 2001
From: Andreas Steinhausen
Date: Wed, 13 Mar 2024 17:07:53 +0100
Subject: [PATCH 01/12] Adapted MemRegion::getDescriptiveName to handle
https://github.com/T-Gruber updated
https://github.com/llvm/llvm-project/pull/85104
>From 0f964127ed91e23f8e969e08ce680535cfeb8906 Mon Sep 17 00:00:00 2001
From: Andreas Steinhausen
Date: Wed, 13 Mar 2024 17:07:53 +0100
Subject: [PATCH 01/12] Adapted MemRegion::getDescriptiveName to handle
https://github.com/NagyDonat approved this pull request.
LGTM.
Using this unit test framework is significantly cleaner than my suggestion that
tries to work around the limitations of ArrayBoundV2.
https://github.com/llvm/llvm-project/pull/85104
___
T-Gruber wrote:
Alright, I added a test case which leads to an incorrect index due to the use
of getOriginRegion. I also added a FIXME comment to document that this case is
not covered currently.
https://github.com/llvm/llvm-project/pull/85104
___
https://github.com/T-Gruber updated
https://github.com/llvm/llvm-project/pull/85104
>From 0f964127ed91e23f8e969e08ce680535cfeb8906 Mon Sep 17 00:00:00 2001
From: Andreas Steinhausen
Date: Wed, 13 Mar 2024 17:07:53 +0100
Subject: [PATCH 01/11] Adapted MemRegion::getDescriptiveName to handle
NagyDonat wrote:
Since suggesting that "matrix" testcase I realized that it won't work, because
`ArrayBoundV2` doesn't perform bounds checking for the lower-dimensional
sub-arrays of a multidimensional array (it only checks that the accessed memory
location is within the full array). For
steakhal wrote:
> Hi @steakhal, just to be sure. We are talking about a test case that covers
> that the index is "recognised" as a wrong variable (the matrix example above)?
Exactly!
https://github.com/llvm/llvm-project/pull/85104
___
cfe-commits
T-Gruber wrote:
Hi @steakhal, just to be sure. We are talking about a test case that covers
that the index is "recognised" as a wrong variable (the matrix example above)?
https://github.com/llvm/llvm-project/pull/85104
___
cfe-commits mailing list
steakhal wrote:
@T-Gruber Could you please add that test case with a FIXME?
https://github.com/llvm/llvm-project/pull/85104
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/NagyDonat edited
https://github.com/llvm/llvm-project/pull/85104
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
https://github.com/NagyDonat approved this pull request.
LGMT, thanks for adding the testcases!
I agree that the "sometimes reports the wrong variable" situation is a very
significant improvement over the old "always infinite loop" situation; but this
"convert a symbol to a variable name"
T-Gruber wrote:
> In addition to the above-mentioned issues there is also a conceptual problem
> with using `getOriginRegion()` to describe a symbol: it names the region
> where the symbol _originated_ (if it originated as the unknown initial value
> of a symbol), which is not necessarily the
@@ -720,14 +720,20 @@ std::string MemRegion::getDescriptiveName(bool UseQuotes)
const {
CI->getValue().toString(Idx);
ArrayIndices = (llvm::Twine("[") + Idx.str() + "]" + ArrayIndices).str();
}
-// If not a ConcreteInt, try to obtain the variable
-//
https://github.com/T-Gruber updated
https://github.com/llvm/llvm-project/pull/85104
>From 0f964127ed91e23f8e969e08ce680535cfeb8906 Mon Sep 17 00:00:00 2001
From: Andreas Steinhausen
Date: Wed, 13 Mar 2024 17:07:53 +0100
Subject: [PATCH 01/10] Adapted MemRegion::getDescriptiveName to handle
steakhal wrote:
> In addition to the above-mentioned issues there is also a conceptual problem
> with using `getOriginRegion()` to describe a symbol: it names the region
> where the symbol _originated_ (if it originated as the unknown initial value
> of a symbol), which is not necessarily the
@@ -720,14 +720,20 @@ std::string MemRegion::getDescriptiveName(bool UseQuotes)
const {
CI->getValue().toString(Idx);
ArrayIndices = (llvm::Twine("[") + Idx.str() + "]" + ArrayIndices).str();
}
-// If not a ConcreteInt, try to obtain the variable
-//
NagyDonat wrote:
In addition to the above-mentioned issues there is also a conceptual problem
with using `getOriginRegion()` to describe a symbol: it names the region where
the symbol _originated_ (if it originated as the unknown initial value of a
symbol), which is not necessarily the region
https://github.com/NagyDonat edited
https://github.com/llvm/llvm-project/pull/85104
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
@@ -720,14 +720,20 @@ std::string MemRegion::getDescriptiveName(bool UseQuotes)
const {
CI->getValue().toString(Idx);
ArrayIndices = (llvm::Twine("[") + Idx.str() + "]" + ArrayIndices).str();
}
-// If not a ConcreteInt, try to obtain the variable
-//
https://github.com/NagyDonat requested changes to this pull request.
I strongly suspect that there are some situations where this commit would
behave incorrectly (assert on legal code or print incorrect output). See inline
comment for explanation and a suggested solution (that I did not test).
https://github.com/NagyDonat edited
https://github.com/llvm/llvm-project/pull/85104
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
T-Gruber wrote:
Thanks again @steakhal! This is helpful and interesting background information.
I really appreciate your help!
https://github.com/llvm/llvm-project/pull/85104
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
steakhal wrote:
> Hello @steakhal, I have just looked through the changes again. What is the
> advantage of using checkPreCall instead of checkLocation? I would very much
> appreciate some background information. Thanks for your help!
Oh yes, I should have explained.
So, `checkLocation` can
T-Gruber wrote:
Hello @steakhal, I have just looked through the changes again. What is the
advantage of using checkPreCall instead of checkLocation? I would very much
appreciate some background information. Thanks for your help!
https://github.com/llvm/llvm-project/pull/85104
https://github.com/steakhal updated
https://github.com/llvm/llvm-project/pull/85104
>From 0f964127ed91e23f8e969e08ce680535cfeb8906 Mon Sep 17 00:00:00 2001
From: Andreas Steinhausen
Date: Wed, 13 Mar 2024 17:07:53 +0100
Subject: [PATCH 1/9] Adapted MemRegion::getDescriptiveName to handle
T-Gruber wrote:
Hi @steakhal, thank you very much for your additional changes!
https://github.com/llvm/llvm-project/pull/85104
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
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 0d98582c8b86644e77f8ddd68fc251e41127b7f4
9c7a82264ffbebc370fdd91f7dc2869c4c488a0b --
steakhal wrote:
I hope you don't mind that I reworked the testing a bit, to be similar to the
rest of the tests.
I also reverted unnecessary clang-formatting where it was not relevant.
I refined the `auto` usage, but other than that I left your code untouched.
As I did everything I thought,
https://github.com/steakhal updated
https://github.com/llvm/llvm-project/pull/85104
>From 0f964127ed91e23f8e969e08ce680535cfeb8906 Mon Sep 17 00:00:00 2001
From: Andreas Steinhausen
Date: Wed, 13 Mar 2024 17:07:53 +0100
Subject: [PATCH 1/8] Adapted MemRegion::getDescriptiveName to handle
32 matches
Mail list logo
