Re: [PATCH] analyzer: Impose recursion limit on indirect calls.
[ Sorry for very late response ] > On 27-Aug-2021, at 6:28 PM, Martin Liška wrote: > > On 8/27/21 14:44, Ankur Saini wrote: >> While working on patch to convert most of the 8 whitespace characters to >> tabs in the analyzer’s source, I see this weird behaviour where at some >> places indentation looks weird when viewed in patch file ( generated via >> “git format-patch” ) but looks alright when viewed in text editor. > > Yes, I can confirm the patch fixed the white spaces, you can verify that with: > $ git diff | ./contrib/check_GNU_style.py - > ... > > Note there are some other formatting issues reported by the script. Yes, I see that too, and have fixed most of them Unfortunately, I can’t seem to find a way to avoid the following report. --- === ERROR type #1: lines should not exceed 80 characters (1 error(s)) === gcc/analyzer/diagnostic-manager.cc:2152:80:= cg_superedge.map_expr_from_caller_to_callee (caller_var, — Also, does these kinds of patches require some special changelog or should I proceed in usual fashion ? == Here is updated patch : == Thanks - Ankur
Re: [PATCH] analyzer: Impose recursion limit on indirect calls.
On 8/27/21 14:44, Ankur Saini wrote: While working on patch to convert most of the 8 whitespace characters to tabs in the analyzer’s source, I see this weird behaviour where at some places indentation looks weird when viewed in patch file ( generated via “git format-patch” ) but looks alright when viewed in text editor. Yes, I can confirm the patch fixed the white spaces, you can verify that with: $ git diff | ./contrib/check_GNU_style.py - ... Note there are some other formatting issues reported by the script. Martin Here is the current patch file, Can someone look at this and see if they also experience the same ?
Re: [PATCH] analyzer: Impose recursion limit on indirect calls.
While working on patch to convert most of the 8 whitespace characters to tabs in the analyzer’s source, I see this weird behaviour where at some places indentation looks weird when viewed in patch file ( generated via “git format-patch” ) but looks alright when viewed in text editor. Here is the current patch file, Can someone look at this and see if they also experience the same ? fix.patch Description: Binary data
Re: [PATCH] analyzer: Impose recursion limit on indirect calls.
On Wed, 2021-08-25 at 21:25 +0530, Ankur Saini wrote:
>
>
> > On 25-Aug-2021, at 8:35 PM, David Malcolm
> > wrote:
> >
> > On Wed, 2021-08-25 at 20:31 +0530, Ankur Saini wrote:
> > >
> > >
> > > > On 25-Aug-2021, at 7:24 PM, Martin Liška
> > > > wrote:
> > > >
> > > > On 8/25/21 15:22, David Malcolm via Gcc-patches wrote:
> > > > > On Wed, 2021-08-25 at 13:39 +0530, Ankur Saini wrote:
> > > > > > This should also fix the failing regression found in PR
> > > > > > analyzer/101980.
> > > > > >
> > > > > > - The patch is in sync with current master
> > > > > > - successfully bootstrapped and tested on x86_64-linux-gnu.
> > > > > >
> > > > > The patch is OK for trunk.
> > > > > Thanks for fixing this
> > > > > Dave
> > > >
> > > > Hello.
> > > >
> > > > Quite accidentally, but I noticed the patch violates GNU coding
> > > > style:
> > > >
> > > > $ git show 43a5d46feabd93ba78983919234f05f5fc9a0982 |
> > > > ./contrib/check_GNU_style.py -
> > > >
> > > > === ERROR type #1: blocks of 8 spaces should be replaced with
> > > > tabs
> > > > (8 error(s)) ===
> > > >
> > > > gcc/analyzer/engine.cc:3064:0: that exceed it further.
> > > >
> > > > gcc/analyzer/engine.cc:3065:0: This is something of a
> > > > blunt
> > > > workaround, but it only
> > > >
> > > > gcc/analyzer/engine.cc:3066:0: applies to recursion
> > > > (and
> > > > mutual recursion), not to
> > > >
> > > > gcc/analyzer/engine.cc:3067:0: general call stacks. */
> > > >
> > > > gcc/analyzer/engine.cc:3069:0: >
> > > > param_analyzer_max_recursion_depth)
> > > >
> > > > gcc/analyzer/engine.cc:3071:0:if (logger)
> > > >
> > > > gcc/analyzer/engine.cc:3072:0: logger->log ("rejecting
> > > > call edge: recursion limit exceeded");
> > > >
> > > > gcc/analyzer/engine.cc:3073:0:return false;
> > > >
> > >
> > > Looks like my editor again converted all the tabs to spaces.
> > >
> > > btw, I can also see a lot of other places where 8 spaces are not
> > > begin converted to tabs, should I also change that accordingly or
> > > leave them the way it is and just update this patch ?
> >
> > It's usually best to split out bugfixes from formatting/whitespace
> > changes, so maybe do it as two patches:
> >
> > (1) an updated version of this patch to fix the recursion issue,
> > using
> > the correct whitespace for the lines that are touched
>
> unfortunately, I already checked in and pushed the changes to the
> master before this was pointed out.
Fair enough.
> But I can add them to the whitespace issue patch.
Sounds good.
Dave
>
> >
> > (2) a patch that fixes whitespaces issues, but doesn't change the
> > behavior of the code
> >
> >
> > Dave
>
Re: [PATCH] analyzer: Impose recursion limit on indirect calls.
> On 25-Aug-2021, at 8:35 PM, David Malcolm wrote:
>
> On Wed, 2021-08-25 at 20:31 +0530, Ankur Saini wrote:
>>
>>
>>> On 25-Aug-2021, at 7:24 PM, Martin Liška wrote:
>>>
>>> On 8/25/21 15:22, David Malcolm via Gcc-patches wrote:
On Wed, 2021-08-25 at 13:39 +0530, Ankur Saini wrote:
> This should also fix the failing regression found in PR
> analyzer/101980.
>
> - The patch is in sync with current master
> - successfully bootstrapped and tested on x86_64-linux-gnu.
>
The patch is OK for trunk.
Thanks for fixing this
Dave
>>>
>>> Hello.
>>>
>>> Quite accidentally, but I noticed the patch violates GNU coding
>>> style:
>>>
>>> $ git show 43a5d46feabd93ba78983919234f05f5fc9a0982 |
>>> ./contrib/check_GNU_style.py -
>>>
>>> === ERROR type #1: blocks of 8 spaces should be replaced with tabs
>>> (8 error(s)) ===
>>>
>>> gcc/analyzer/engine.cc:3064:0: that exceed it further.
>>>
>>> gcc/analyzer/engine.cc:3065:0: This is something of a blunt
>>> workaround, but it only
>>>
>>> gcc/analyzer/engine.cc:3066:0: applies to recursion (and
>>> mutual recursion), not to
>>>
>>> gcc/analyzer/engine.cc:3067:0: general call stacks. */
>>>
>>> gcc/analyzer/engine.cc:3069:0: >
>>> param_analyzer_max_recursion_depth)
>>>
>>> gcc/analyzer/engine.cc:3071:0:if (logger)
>>>
>>> gcc/analyzer/engine.cc:3072:0: logger->log ("rejecting
>>> call edge: recursion limit exceeded");
>>>
>>> gcc/analyzer/engine.cc:3073:0:return false;
>>>
>>
>> Looks like my editor again converted all the tabs to spaces.
>>
>> btw, I can also see a lot of other places where 8 spaces are not
>> begin converted to tabs, should I also change that accordingly or
>> leave them the way it is and just update this patch ?
>
> It's usually best to split out bugfixes from formatting/whitespace
> changes, so maybe do it as two patches:
>
> (1) an updated version of this patch to fix the recursion issue, using
> the correct whitespace for the lines that are touched
unfortunately, I already checked in and pushed the changes to the master before
this was pointed out. But I can add them to the whitespace issue patch.
>
> (2) a patch that fixes whitespaces issues, but doesn't change the
> behavior of the code
>
>
> Dave
Re: [PATCH] analyzer: Impose recursion limit on indirect calls.
On Wed, 2021-08-25 at 20:31 +0530, Ankur Saini wrote:
>
>
> > On 25-Aug-2021, at 7:24 PM, Martin Liška wrote:
> >
> > On 8/25/21 15:22, David Malcolm via Gcc-patches wrote:
> > > On Wed, 2021-08-25 at 13:39 +0530, Ankur Saini wrote:
> > > > This should also fix the failing regression found in PR
> > > > analyzer/101980.
> > > >
> > > > - The patch is in sync with current master
> > > > - successfully bootstrapped and tested on x86_64-linux-gnu.
> > > >
> > > The patch is OK for trunk.
> > > Thanks for fixing this
> > > Dave
> >
> > Hello.
> >
> > Quite accidentally, but I noticed the patch violates GNU coding
> > style:
> >
> > $ git show 43a5d46feabd93ba78983919234f05f5fc9a0982 |
> > ./contrib/check_GNU_style.py -
> >
> > === ERROR type #1: blocks of 8 spaces should be replaced with tabs
> > (8 error(s)) ===
> >
> > gcc/analyzer/engine.cc:3064:0: that exceed it further.
> >
> > gcc/analyzer/engine.cc:3065:0: This is something of a blunt
> > workaround, but it only
> >
> > gcc/analyzer/engine.cc:3066:0: applies to recursion (and
> > mutual recursion), not to
> >
> > gcc/analyzer/engine.cc:3067:0: general call stacks. */
> >
> > gcc/analyzer/engine.cc:3069:0: >
> > param_analyzer_max_recursion_depth)
> >
> > gcc/analyzer/engine.cc:3071:0:if (logger)
> >
> > gcc/analyzer/engine.cc:3072:0: logger->log ("rejecting
> > call edge: recursion limit exceeded");
> >
> > gcc/analyzer/engine.cc:3073:0:return false;
> >
>
> Looks like my editor again converted all the tabs to spaces.
>
> btw, I can also see a lot of other places where 8 spaces are not
> begin converted to tabs, should I also change that accordingly or
> leave them the way it is and just update this patch ?
It's usually best to split out bugfixes from formatting/whitespace
changes, so maybe do it as two patches:
(1) an updated version of this patch to fix the recursion issue, using
the correct whitespace for the lines that are touched
(2) a patch that fixes whitespaces issues, but doesn't change the
behavior of the code
Dave
Re: [PATCH] analyzer: Impose recursion limit on indirect calls.
On 8/25/21 15:22, David Malcolm via Gcc-patches wrote:
On Wed, 2021-08-25 at 13:39 +0530, Ankur Saini wrote:
This should also fix the failing regression found in PR
analyzer/101980.
- The patch is in sync with current master
- successfully bootstrapped and tested on x86_64-linux-gnu.
The patch is OK for trunk.
Thanks for fixing this
Dave
Hello.
Quite accidentally, but I noticed the patch violates GNU coding style:
$ git show 43a5d46feabd93ba78983919234f05f5fc9a0982 |
./contrib/check_GNU_style.py -
=== ERROR type #1: blocks of 8 spaces should be replaced with tabs (8 error(s))
===
gcc/analyzer/engine.cc:3064:0: that exceed it further.
gcc/analyzer/engine.cc:3065:0: This is something of a blunt workaround,
but it only
gcc/analyzer/engine.cc:3066:0: applies to recursion (and mutual
recursion), not to
gcc/analyzer/engine.cc:3067:0: general call stacks. */
gcc/analyzer/engine.cc:3069:0: > param_analyzer_max_recursion_depth)
gcc/analyzer/engine.cc:3071:0:if (logger)
gcc/analyzer/engine.cc:3072:0: logger->log ("rejecting call edge: recursion
limit exceeded");
gcc/analyzer/engine.cc:3073:0:return false;
Martin
Re: [PATCH] analyzer: Impose recursion limit on indirect calls.
On Wed, 2021-08-25 at 13:39 +0530, Ankur Saini wrote: > This should also fix the failing regression found in PR > analyzer/101980. > > - The patch is in sync with current master > - successfully bootstrapped and tested on x86_64-linux-gnu. > The patch is OK for trunk. Thanks for fixing this Dave
[PATCH] analyzer: Impose recursion limit on indirect calls.
This should also fix the failing regression found in PR analyzer/101980. - The patch is in sync with current master - successfully bootstrapped and tested on x86_64-linux-gnu. fix.patch Description: Binary data Thanks - Ankur
