Re: Possible suboptimal code generated in 32-bit ABI mode
On Sun, Nov 19, 2017 at 5:35 PM, Richard Bradfield wrote: > On Sun, Nov 19, 2017 at 04:19:45PM +, bradf...@fstab.me wrote: >> >> For reference, I am compiling everything using gcc trunk, at commit >> 254929 from Sun Nov 19, and I am benchmarking on a Skylake i7-6700K >> at 4.0GHz. > > > And something else I should definitely have included in the first place, > I cannot reproduce this on the GCC 7.2 shipped with my distribution, so > is it possible this is a regression? Please open a bugreport. Thanks, Richard. > Richard
Re: Finding all CALL_EXPR tree nodes during GENERIC
On Sun, Nov 19, 2017 at 9:27 PM, Katsunori Kumatani wrote: > Yes, it seems walk_tree does the job, I was probably doing something > wrong before. Thank you. > > I have a question about it. Am I correct in assuming I only have to > check the subtrees of nodes that satisfy EXPR_P? So for nodes that > aren't EXPR_P, can I just set *walkSubtrees to 0, or will that miss > some CALL_EXPRs? In my tests, none were missed, but I don't know if > this is all-encompassing for C/C++ generated AST. You'd have to experiment, it depends on the C/C++ AST. You should be able to skip TYPE_P though. > Oh and another thing, is it possible to change the node (or create a > new one and modify the source that points to it)? I assume that's why > the first parameter to walk_tree_fn is a tree* right? That would be > useful to know too. Yes, you can change things. Richard. > On Sat, Nov 18, 2017 at 6:49 PM, Richard Biener > wrote: >> On November 18, 2017 5:38:35 PM GMT+01:00, Katsunori Kumatani >> wrote: >>>Hello, I'm doing some testing with a gcc plugin (also helped me >>>understand a lot of how GCC works) but I've hit a roadblock. >>> >>>I want to find all of a function body's CALL_EXPR nodes (i.e. what >>>calls it has) early during GENERIC, so I assume the >>>PLUGIN_PRE_GENERICIZE hook is what I should use right? >>> >>>Anyway, my problem is I don't know how to find all the CALL_EXPR nodes >>>from the DECL_SAVED_TREE of the function's decl. So what's the >>>simplest way to do that? The routines in tree-iterator.h don't help >>>much, they only iterate through a statement list. Do I have to go >>>through every expr manually? Is there no API available? (walk_tree >>>doesn't seem to handle it, or maybe I don't know how to use it) >> >> Walk_tree should do it. >> >> Richard. >> >>> >>>This is obviously trivial to do in GIMPLE form (iterating through all >>>statements and finding GIMPLE_CALL) but I need it as early as >>>possible, so that would be in the tree representation (GENERIC?). I >>>hope I'm just missing something obvious. >>> >>>Thanks. >>
GCC 8.0.0 Status Report (2017-11-20)
Status == GCC 8 is now in Stage 3 which means open for general bugfixing. As usual we're now in a short period where posted but not yet reviewed feature patches can be accepted. As time goes by even those will be no longer appropriate. As usual in this time not all regressions have been prioritized, usual rules apply -- regressions new in GCC 8 will end up as P1 unless they do not affect primary or secondary targets or languages. Regressions that we shipped with in GCC 7.2 can only be at most P2. Regressions that only affect non-primary/secondary targets or languages will be demoted to P4/5. Quality Data Priority # Change from last report --- --- P1 14 P2 161 - 2 P3 163 + 29 P4 134 - 1 P5 27 --- --- Total P1-P3 338 + 27 Total 499 + 26 Previous Report === https://gcc.gnu.org/ml/gcc/2017-11/msg00021.html
Re: Please review writeup for fixing PR 78809 (inline strcmp for small constant strings)
> On Nov 17, 2017, at 5:54 PM, Martin Sebor wrote:
>
>>
>> for the safety checking purpose, when we try to convert
>>
>> __builtin_strcmp(s, "abc") != 0
>>
>> to
>>
>> __builtin_memcmp (s, “abc”, 4) != 0
>>
>> we have to make sure that the size of variable “s” is larger than “4”.
>
> Presumably you mean "is at least 4?”
Yes.:-)
>
>>
>> if “s” is declared as
>>
>> char s[100];
>>
>> currently, the “get_range_strlen” cannot determine its maximum length is
>> 100. (it just return UNKNOWN).
>>
>> so, I have to update “get_range_strlen” for such simple case.
>>
>> this does provide the information I want. However, since the routine
>> “get_range_strlen” is also used in other places,
>> for example, in gimple-ssa-sprintf.c, the implementation of the sprintf
>> overflow warning uses the routine “get_range_strlen”
>> to decide the string’s maximum size and buffer size.
>>
>> my change in “get_range_strlen” triggered some new warnings for
>> -Werror=format-overflow (from gimple-ssa-sprintf.c
>> mentioned above) as following:
>>
>> qinzhao@gcc116:~/Bugs/warning$ cat t.c
>> #include
>>
>> void foo(const char *macro)
>> {
>> char buf1[256], buf2[256];
>> sprintf (buf1, "%s=%s", macro, buf2);
>> return;
>> }
>>
>> with my private GCC:
>>
>> qinzhao@gcc116:~/Bugs/warning$ /home/qinzhao/Install/latest/bin/gcc t.c
>> -Werror=format-overflow -S
>> t.c: In function ‘foo’:
>> t.c:6:18: error: ‘sprintf’ may write a terminating nul past the end of the
>> destination [-Werror=format-overflow=]
>> sprintf (buf1, "%s=%s", macro, buf2);
>> ^~~
>> t.c:6:3: note: ‘sprintf’ output 2 or more bytes (assuming 257) into a
>> destination of size 256
>> sprintf (buf1, "%s=%s", macro, buf2);
>> ^~~~
>> cc1: some warnings being treated as errors
>
> When the length of one or more of the strings referenced by
> the argument passed to get_range_strlen() is unknown
> the -Wformat-overflow checker uses get_range_strlen() to compute
> the length of the longest string that fits in an array reference
> by the subexpression (i.e., sizeof array - 1) and uses it to
> issue warnings. This works with member arrays but because of
> a bug/limitation it doesn't work for non-member arrays. Bug
> 79538 tracks this. So the warning above suggests your change
> has fixed the problem -- good work! :)
really thanks for the info and bug id.
I just checked the 2 testing cases in PR 79538, with my private GCC, both of
the warnings are reported.
I am assign this bug to myself too.
Qing
>
> Martin
futurist.co.in
Hi , We are own this domain name, have you made a decision to purchase it ? Looking forward to your reply. My admin e-mail: 2398...@qq.com Best regards, Alice
Re: Please review writeup for fixing PR 78809 (inline strcmp for small constant strings)
> On Nov 17, 2017, at 7:32 PM, Jeff Law wrote: > > On 11/17/2017 03:45 PM, Qing Zhao wrote: do you think using this routine is good? or do you have other suggestions (since I am still not very familiar with the internals of GCC, might not find the best available one now…) >>> The range information attached to an SSA_NAME is global data. ie, it >>> must hold at all locations where the object in question might be >>> referenced. This implies that it will sometimes (often?) be less >>> precise than you might like. >> >> do you mean the “value_range” attached to SSA_NAME? >> >> For my purpose, I’d like to get the maximum length of char array s[100] is >> 100, which is larger than the size of constant string “abc”, then >> I can safely apply the transformation to memcmp. >> >> can “value_range” info serve this purpose? > No it can't. Sorry for leading you the wrong direction. What you're > looking for is the object size interfaces. > > See tree-object-size.[ch] > > That's a pass that tries to compute the sizes of various objects > referenced by the IL. > > Note that the object size is different than say the length of a string > stored in an object for which you'll probably be looking at > tree-ssa-strlen's interfaces. thanks for the info. Yes, during my current implementation for B, I tried to use the interface for “tree-object-size”, i.e, “compute_builtin_object_size” to decide the maximum length of the arrays, I noticed that it did not provide the information for the simple cases. then I switched to use “get_range_strlen”. I will double check on this. > Ranges are more for integer objects. ie, i has the value [0,25] or ~[0,0]. Okay, I see. > >>> >>> I am currently working towards an embeddable context sensitive range >>> analyzer that in theory could be used within tree-ssa-strlen pass to >>> give more precise range information. I'm hoping to wrap that work up in >>> the next day or so so that folks can use it in gcc-8. >> >> such context sensitive range info should be useful when we relax the >> constant “N” to be an expression whose Min value is larger than the length >> of constant string, with it, we can catch more opportunities. >> let me know when this info is available. > Hoping to have the basics into the trunk within the next few days as > reviews flow in. thanks. Qing > > jeff
Fail to compile GDB with recent GCC trunk (-Werror=stringop-overflow=, -Werror=stringop-truncation)
Hi,
I failed to compile GDB with GCC trunk (8.0.0 20171117) because of some
-Werror=stringop-overflow= and -Werror=stringop-truncation warnings.
Some of them are not necessary to me,
1. ../../binutils-gdb/gdb/python/py-gdb-readline.c:79:15: error: ‘char*
strncpy(char*, const char*, size_t)’ output truncated before terminating nul
copying as many bytes from a string as its length [-Werror=stringop-truncation]
strncpy (q, p, n);
^
../../binutils-gdb/gdb/python/py-gdb-readline.c:73:14: note: length computed
here
n = strlen (p);
~~~^~~
the code is simple,
n = strlen (p);
/* Copy the line to Python and return. */
q = (char *) PyMem_RawMalloc (n + 2);
if (q != NULL)
{
strncpy (q, p, n);
q[n] = '\n';
q[n + 1] = '\0';
}
I don't see the point of warning here.
2. ../../binutils-gdb/gdb/cp-namespace.c:1071:11: error: ‘char* strncpy(char*,
const char*, size_t)’ output truncated before terminating nul copying 2 bytes
from a string of the same length [-Werror=stringop-truncation]
strncpy (full_name + scope_length, "::", 2);
^~~
full_name = (char *) alloca (scope_length + 2 + strlen (name) + 1);
strncpy (full_name, scope, scope_length);
strncpy (full_name + scope_length, "::", 2);
strcpy (full_name + scope_length + 2, name);
the code looks right to me,
Likewise,
../../../binutils-gdb/gdb/gdbserver/remote-utils.c:1204:14: error: ‘char*
strncpy(char*, const char*, size_t)’ output truncated before terminating nul
copying 6 bytes from a string of the same length [-Werror=stringop-truncation]
strncpy (buf, "watch:", 6);
^~
strncpy (buf, "watch:", 6);
buf += 6;
*buf = '\0';
I can "fix" these warnings by changing GDB code, use strcpy in 1) and
use memcpy in 2). Do we expect all the users of GCC 8 changing their
correct code because GCC is not happy on the code? The warning is too
aggressive to me.
--
Yao (齐尧)
Re: Please review writeup for fixing PR 78809 (inline strcmp for small constant strings)
> On Nov 17, 2017, at 7:39 PM, Jeff Law wrote: > >>> >> >> thanks for the info, Martin. >> >> In my case, it’s the size of “100” cannot be collected in the >> MINMAXLEN[1] for the string “s”. >> >> I need to make sure that the size of variable string s is larger than >> the size of constant string “abc” to guarantee the safety of the >> transformation. >> >> currently, “get_range_strlen” cannot identify the simple VAR_DECL with >> array_type to determine the maximum size of the string. > It sounds more like you want the object_size interfaces. See > tree-object-size.[ch] As I mentioned in the other email, I tried the interface of object_size, i.e, “compute_builtin_object_size” to see whether it can provide me good info on several simple examples, at that time, I didn’t see that it can provide good info for very simple cases, so I switched to use “get_range_strlen” and modified it to serve my purpose. I will double check on this. Qing > Jeff
Re: Fail to compile GDB with recent GCC trunk (-Werror=stringop-overflow=, -Werror=stringop-truncation)
On 11/20/17, Yao Qi wrote:
>
> Hi,
> I failed to compile GDB with GCC trunk (8.0.0 20171117) because of some
> -Werror=stringop-overflow= and -Werror=stringop-truncation warnings.
> Some of them are not necessary to me,
>
> 1. ../../binutils-gdb/gdb/python/py-gdb-readline.c:79:15: error: ‘char*
> strncpy(char*, const char*, size_t)’ output truncated before terminating nul
> copying as many bytes from a string as its length
> [-Werror=stringop-truncation]
>strncpy (q, p, n);
>^
> ../../binutils-gdb/gdb/python/py-gdb-readline.c:73:14: note: length computed
> here
>n = strlen (p);
>~~~^~~
>
> the code is simple,
>
> n = strlen (p);
>
> /* Copy the line to Python and return. */
> q = (char *) PyMem_RawMalloc (n + 2);
> if (q != NULL)
> {
> strncpy (q, p, n);
> q[n] = '\n';
> q[n + 1] = '\0';
> }
>
> I don't see the point of warning here.
>
> 2. ../../binutils-gdb/gdb/cp-namespace.c:1071:11: error: ‘char*
> strncpy(char*, const char*, size_t)’ output truncated before terminating nul
> copying 2 bytes from a string of the same length
> [-Werror=stringop-truncation]
>strncpy (full_name + scope_length, "::", 2);
>^~~
>
> full_name = (char *) alloca (scope_length + 2 + strlen (name) + 1);
> strncpy (full_name, scope, scope_length);
> strncpy (full_name + scope_length, "::", 2);
> strcpy (full_name + scope_length + 2, name);
>
> the code looks right to me,
>
> Likewise,
>
> ../../../binutils-gdb/gdb/gdbserver/remote-utils.c:1204:14: error: ‘char*
> strncpy(char*, const char*, size_t)’ output truncated before terminating nul
> copying 6 bytes from a string of the same length
> [-Werror=stringop-truncation]
> strncpy (buf, "watch:", 6);
> ^~
>
> strncpy (buf, "watch:", 6);
> buf += 6;
>
> *buf = '\0';
>
> I can "fix" these warnings by changing GDB code, use strcpy in 1) and
> use memcpy in 2). Do we expect all the users of GCC 8 changing their
> correct code because GCC is not happy on the code? The warning is too
> aggressive to me.
>
> --
> Yao (齐尧)
>
I thought there was a gcc bug open about this but now I can't seem to
find it; please let me know if you come across the one I was trying to
remember...
Re: Fail to compile GDB with recent GCC trunk (-Werror=stringop-overflow=, -Werror=stringop-truncation)
On 11/20/2017 08:51 AM, Yao Qi wrote:
Hi,
I failed to compile GDB with GCC trunk (8.0.0 20171117) because of some
-Werror=stringop-overflow= and -Werror=stringop-truncation warnings.
Some of them are not necessary to me,
I have the attached patch for two of these but I have been waiting
to submit it until the latest GCC patch has been approved that
adjusts the checker a bit.
1. ../../binutils-gdb/gdb/python/py-gdb-readline.c:79:15: error: ‘char*
strncpy(char*, const char*, size_t)’ output truncated before terminating nul
copying as many bytes from a string as its length [-Werror=stringop-truncation]
strncpy (q, p, n);
^
../../binutils-gdb/gdb/python/py-gdb-readline.c:73:14: note: length computed
here
n = strlen (p);
~~~^~~
the code is simple,
n = strlen (p);
/* Copy the line to Python and return. */
q = (char *) PyMem_RawMalloc (n + 2);
if (q != NULL)
{
strncpy (q, p, n);
q[n] = '\n';
q[n + 1] = '\0';
}
I don't see the point of warning here.
The overall purpose of the warning is to help find likely misuses
of strncpy and strncat. As with any warning that's based on intent,
it cannot avoid highlighting some safe uses, or missing some unsafe
ones.
The case above is based on a heuristic designed to find bugs where
the bound depends on the length of the source rather the size of
the destination, as in:
strncpy (d, s, strlen (s));
This is, unfortunately, a common misuse/mistake. It's often seen
in legacy code that's being updated in response to a security
mandate to replace strcpy with strncpy.
The GDB use case, although safe, is also not how the function is
intended to be used. The intended use is to specify the size of
the destination, typically a statically allocated array, and have
the function fill it with data (not necessarily a string, and
not necessarily containing a terminating nul). When the array
is allocated dynamically and sized to store the entire string
it's preferable to use some other function (e.g., memcpy or
strcpy).
2. ../../binutils-gdb/gdb/cp-namespace.c:1071:11: error: ‘char* strncpy(char*,
const char*, size_t)’ output truncated before terminating nul copying 2 bytes
from a string of the same length [-Werror=stringop-truncation]
strncpy (full_name + scope_length, "::", 2);
^~~
full_name = (char *) alloca (scope_length + 2 + strlen (name) + 1);
strncpy (full_name, scope, scope_length);
strncpy (full_name + scope_length, "::", 2);
This is safe, although also not the intended use of the function.
The call above can be replaced either by memcpy or strcpy. There
also is no good way to avoid warning on it without compromising
the efficacy of the checker.
strcpy (full_name + scope_length + 2, name);
the code looks right to me,
Likewise,
../../../binutils-gdb/gdb/gdbserver/remote-utils.c:1204:14: error: ‘char*
strncpy(char*, const char*, size_t)’ output truncated before terminating nul
copying 6 bytes from a string of the same length [-Werror=stringop-truncation]
strncpy (buf, "watch:", 6);
^~
strncpy (buf, "watch:", 6);
buf += 6;
*buf = '\0';
As above, memcpy or strcpy are the preferred alternatives.
Martin
--- Begin Message ---
The latest revision of GCC 8.0 adds a -Wstringop-truncation option
to detect common misuses of the strncpy and strncat functions that
may truncate the copy and leave the result without a terminating
nul character.
In testing the implementation with GDB sources on x86_64 I found
a few instances of the warning that are issued for what's safe
but nevertheless not strictly intended uses of the functions
(i.e., to create "bounded" non-nul-terminated copies of a string).
I adjusted the warning to accept some but not all of these use
cases. The attached patch shows the two instances of the warning
that I had to suppress in GDB.
In general, even though the checker handles some such cases, to
avoid the warning, it's best to use strncpy only with a bound that
reflects the size of the destination, never that of the source.
Martin
2017-11-10 Martin Sebor
* gdb/cli/cli-decode.c (help_list): Use strcpy and memcpy instead
of strncpy.
* gdb/cp-namespace.c (cp_lookup_transparent_type_loop): Use strcpy
instead of strncpy to avoid -Wstringop-truncation.
* gdb/gdbserver/remote-utils.c (prepare_resume_reply): Use memcpy
instead of strncpy to avoid -Wstringop-truncation.
diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 87ebed5..968779e 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -1,4 +1,4 @@
-/* Handle lists of commands, their decoding and documentation, for GDB.
+t/* Handle lists of commands, their decoding and documentation, for GDB.
Copyright (C) 1986-2017 Free Software Foundation, Inc.
@@ -1115,9 +1115,8 @@ help_list (struct cmd_list_element *list, const char *cmdtype,
if (len)
Re: Fail to compile GDB with recent GCC trunk (-Werror=stringop-overflow=, -Werror=stringop-truncation)
On 17-11-20 09:33:46, Martin Sebor wrote:
> On 11/20/2017 08:51 AM, Yao Qi wrote:
> >
> >Hi,
> >I failed to compile GDB with GCC trunk (8.0.0 20171117) because of some
> >-Werror=stringop-overflow= and -Werror=stringop-truncation warnings.
> >Some of them are not necessary to me,
>
> I have the attached patch for two of these but I have been waiting
> to submit it until the latest GCC patch has been approved that
> adjusts the checker a bit.
Hi Martin,
can you post the patch to gdb-patc...@sourceware.org? The patch can
be reviewed there.
>
> >
> >1. ../../binutils-gdb/gdb/python/py-gdb-readline.c:79:15: error: ‘char*
> >strncpy(char*, const char*, size_t)’ output truncated before terminating nul
> >copying as many bytes from a string as its length
> >[-Werror=stringop-truncation]
> > strncpy (q, p, n);
> > ^
> >../../binutils-gdb/gdb/python/py-gdb-readline.c:73:14: note: length computed
> >here
> > n = strlen (p);
> > ~~~^~~
> >
> >the code is simple,
> >
> > n = strlen (p);
> >
> > /* Copy the line to Python and return. */
> > q = (char *) PyMem_RawMalloc (n + 2);
> > if (q != NULL)
> >{
> > strncpy (q, p, n);
> > q[n] = '\n';
> > q[n + 1] = '\0';
> >}
> >
> >I don't see the point of warning here.
>
> The overall purpose of the warning is to help find likely misuses
> of strncpy and strncat. As with any warning that's based on intent,
> it cannot avoid highlighting some safe uses, or missing some unsafe
> ones.
As a user, false negative is fine to me, but false positive is too noisy.
If I made stupid mistakes and compiler doesn't find them, that is fine.
The people who wrote such bad code should be blamed, rather than
compiler. However, if compiler emits many warnings to the correct code,
it is annoying. Usually, "too many false warnings" == "no warnings".
>
> The case above is based on a heuristic designed to find bugs where
> the bound depends on the length of the source rather the size of
> the destination, as in:
>
> strncpy (d, s, strlen (s));
>
> This is, unfortunately, a common misuse/mistake. It's often seen
> in legacy code that's being updated in response to a security
> mandate to replace strcpy with strncpy.
>
> The GDB use case, although safe, is also not how the function is
> intended to be used. The intended use is to specify the size of
> the destination, typically a statically allocated array, and have
> the function fill it with data (not necessarily a string, and
> not necessarily containing a terminating nul). When the array
> is allocated dynamically and sized to store the entire string
> it's preferable to use some other function (e.g., memcpy or
> strcpy).
The real issue here is GCC warning is too aggressive, and even emit
warnings to the correct code. We fixed glibc, gdb, what is the next?
GCC will be used to build thousands of packages, do you expect "fixing"
all of them? If I have to fix my correct code, just because gcc
complains about it, it is time I consider switching to other compilers
to compile my code.
>
> >
> >2. ../../binutils-gdb/gdb/cp-namespace.c:1071:11: error: ‘char*
> >strncpy(char*, const char*, size_t)’ output truncated before terminating nul
> >copying 2 bytes from a string of the same length
> >[-Werror=stringop-truncation]
> > strncpy (full_name + scope_length, "::", 2);
> > ^~~
> >
> > full_name = (char *) alloca (scope_length + 2 + strlen (name) + 1);
> > strncpy (full_name, scope, scope_length);
> > strncpy (full_name + scope_length, "::", 2);
>
> This is safe, although also not the intended use of the function.
> The call above can be replaced either by memcpy or strcpy. There
> also is no good way to avoid warning on it without compromising
> the efficacy of the checker.
>
IMO, compiler != static analysis/checker, although they share many
technologies. They have different responsibilities, compiler is to
generate binary code from source code, while static analysis tool
is to find problems in the code as many as possible.
--
Yao (齐尧)
