Re: [PATCH] Check \0-termination of string in c_getstr (simplified version)
On 10/14/2016 11:38 AM, Richard Biener wrote: > On Thu, Oct 13, 2016 at 5:23 PM, Martin Liška wrote: >> Hello. >> >> After receiving feedback from Richi and Wilco Dijkstra, I decided to fully >> not >> support not null-terminated strings. It brings more complications and the >> code has started >> to be overengineered. Thus c_getstr accepts only such strings and as a bonus >> it returns >> length of a string. >> >> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. >> >> Ready to be installed? > > + /* Support only properly null-terminated strings. */ > + if (string_length == 0 > + || string[string_length - 1] != '\0' > + || offset > string_length) > > shouldn't this be offset >= string_length? Yes, it should be that, installed as r241152. Thanks, Martin > > Ok with that change. > > Thanks, > Richard. > >> Martin
Re: [PATCH] Check \0-termination of string in c_getstr (simplified version)
On Thu, Oct 13, 2016 at 5:23 PM, Martin Liška wrote: > Hello. > > After receiving feedback from Richi and Wilco Dijkstra, I decided to fully not > support not null-terminated strings. It brings more complications and the > code has started > to be overengineered. Thus c_getstr accepts only such strings and as a bonus > it returns > length of a string. > > Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. > > Ready to be installed? + /* Support only properly null-terminated strings. */ + if (string_length == 0 + || string[string_length - 1] != '\0' + || offset > string_length) shouldn't this be offset >= string_length? Ok with that change. Thanks, Richard. > Martin
[PATCH] Check \0-termination of string in c_getstr (simplified version)
Hello. After receiving feedback from Richi and Wilco Dijkstra, I decided to fully not support not null-terminated strings. It brings more complications and the code has started to be overengineered. Thus c_getstr accepts only such strings and as a bonus it returns length of a string. Patch can bootstrap on ppc64le-redhat-linux and survives regression tests. Ready to be installed? Martin >From bee44f0dedc86b1c354e21dd87dad6313147dcc3 Mon Sep 17 00:00:00 2001 From: marxin Date: Thu, 13 Oct 2016 10:20:12 +0200 Subject: [PATCH 1/4] Support only \0-terminated string in c_getstr and return strlen gcc/ChangeLog: 2016-10-13 Martin Liska * fold-const.c (c_getstr): Support of properly \0-terminated string constants. New argument is added. * fold-const.h: New argument is added. --- gcc/fold-const.c | 38 +- gcc/fold-const.h | 2 +- 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/gcc/fold-const.c b/gcc/fold-const.c index 02aa484..57a9243 100644 --- a/gcc/fold-const.c +++ b/gcc/fold-const.c @@ -14440,24 +14440,44 @@ fold_build_pointer_plus_hwi_loc (location_t loc, tree ptr, HOST_WIDE_INT off) } /* Return a char pointer for a C string if it is a string constant - or sum of string constant and integer constant. */ + or sum of string constant and integer constant. We only support + string constants properly terminated with '\0' character. + If STRLEN is a valid pointer, length (including terminating character) + of returned string is stored to the argument. */ const char * -c_getstr (tree src) +c_getstr (tree src, unsigned HOST_WIDE_INT *strlen) { tree offset_node; + if (strlen) +*strlen = 0; + src = string_constant (src, &offset_node); if (src == 0) -return 0; +return NULL; - if (offset_node == 0) -return TREE_STRING_POINTER (src); - else if (!tree_fits_uhwi_p (offset_node) - || compare_tree_int (offset_node, TREE_STRING_LENGTH (src) - 1) > 0) -return 0; + unsigned HOST_WIDE_INT offset = 0; + if (offset_node != NULL_TREE) +{ + if (!tree_fits_uhwi_p (offset_node)) + return NULL; + else + offset = tree_to_uhwi (offset_node); +} + + unsigned HOST_WIDE_INT string_length = TREE_STRING_LENGTH (src); + const char *string = TREE_STRING_POINTER (src); + + /* Support only properly null-terminated strings. */ + if (string_length == 0 + || string[string_length - 1] != '\0' + || offset > string_length) +return NULL; - return TREE_STRING_POINTER (src) + tree_to_uhwi (offset_node); + if (strlen) +*strlen = string_length - offset; + return string + offset; } #if CHECKING_P diff --git a/gcc/fold-const.h b/gcc/fold-const.h index 637e46b..bc22c88 100644 --- a/gcc/fold-const.h +++ b/gcc/fold-const.h @@ -182,7 +182,7 @@ extern bool expr_not_equal_to (tree t, const wide_int &); extern tree const_unop (enum tree_code, tree, tree); extern tree const_binop (enum tree_code, tree, tree, tree); extern bool negate_mathfn_p (combined_fn); -extern const char *c_getstr (tree); +extern const char *c_getstr (tree, unsigned HOST_WIDE_INT *strlen = NULL); /* Return OFF converted to a pointer offset type suitable as offset for POINTER_PLUS_EXPR. Use location LOC for this conversion. */ -- 2.9.2
Re: [PATCH] Check \0-termination of string in c_getstr
On 10/11/2016 12:28 PM, Richard Biener wrote: > On Tue, Oct 11, 2016 at 11:27 AM, Martin Liška wrote: >> As mentioned in the email that I reply to, c_getstr should check >> null termination of string constants. >> >> Tests of the whole series have been running. > > Looks ok to me (if testing passes). Thanks for the review, however I decided to enhance the API to support a requested length argument (if a string is not null terminated) and to return length of the string(usable for strn* functions folding). Patch can bootstrap on ppc64le-redhat-linux and survives regression tests as a whole series. Martin > > Thanks, > Richard. > >> Thanks, >> Martin >From e6c16ea038104ef15b087ff9fca23d9b2406e65e Mon Sep 17 00:00:00 2001 From: marxin Date: Mon, 10 Oct 2016 12:13:12 +0200 Subject: [PATCH] Enhance c_getstr API gcc/ChangeLog: 2016-10-10 Martin Liska * fold-const.c (c_getstr): Guard string termination, or validate that requested length is within a string constant. * fold-const.h (c_getstr): Set default value for the new arg. --- gcc/fold-const.c | 44 +++- gcc/fold-const.h | 3 ++- 2 files changed, 37 insertions(+), 10 deletions(-) diff --git a/gcc/fold-const.c b/gcc/fold-const.c index 02aa484..eb53e84 100644 --- a/gcc/fold-const.c +++ b/gcc/fold-const.c @@ -14440,24 +14440,50 @@ fold_build_pointer_plus_hwi_loc (location_t loc, tree ptr, HOST_WIDE_INT off) } /* Return a char pointer for a C string if it is a string constant - or sum of string constant and integer constant. */ + or sum of string constant and integer constant. + If the string constant is properly zero-terminated, the constant is returned. + Otherwise, if REQ_LENGTH is a non-negative number, the constant + is returned if the string length is greater or equal to REQ_LENGTH. + If STRLEN is a valid pointer, length (including terminatinch character) + of returned string is stored to the argument. */ const char * -c_getstr (tree src) +c_getstr (tree src, HOST_WIDE_INT req_length, unsigned HOST_WIDE_INT *strlen) { tree offset_node; + if (strlen) +*strlen = 0; + src = string_constant (src, &offset_node); if (src == 0) -return 0; +return NULL; - if (offset_node == 0) -return TREE_STRING_POINTER (src); - else if (!tree_fits_uhwi_p (offset_node) - || compare_tree_int (offset_node, TREE_STRING_LENGTH (src) - 1) > 0) -return 0; + unsigned HOST_WIDE_INT offset = 0; + if (offset_node != NULL_TREE) +{ + if (!tree_fits_uhwi_p (offset_node)) + return NULL; + else + offset = tree_to_uhwi (offset_node); +} - return TREE_STRING_POINTER (src) + tree_to_uhwi (offset_node); + unsigned HOST_WIDE_INT string_length = TREE_STRING_LENGTH (src); + const char *string = TREE_STRING_POINTER (src); + if (offset > string_length) +return NULL; + + /* If the string is properly '\0' character terminated, return it. */ + if ((string_length > 0 && string[string_length - 1] == 0) + || (req_length != -1 + && (unsigned HOST_WIDE_INT)req_length <= string_length - offset)) +{ + if (strlen) + *strlen = string_length - offset; + return string + offset; +} + else +return NULL; } #if CHECKING_P diff --git a/gcc/fold-const.h b/gcc/fold-const.h index 637e46b..bbf831a 100644 --- a/gcc/fold-const.h +++ b/gcc/fold-const.h @@ -182,7 +182,8 @@ extern bool expr_not_equal_to (tree t, const wide_int &); extern tree const_unop (enum tree_code, tree, tree); extern tree const_binop (enum tree_code, tree, tree, tree); extern bool negate_mathfn_p (combined_fn); -extern const char *c_getstr (tree); +extern const char *c_getstr (tree src, HOST_WIDE_INT req_length = -1, + unsigned HOST_WIDE_INT *strlen = NULL); /* Return OFF converted to a pointer offset type suitable as offset for POINTER_PLUS_EXPR. Use location LOC for this conversion. */ -- 2.9.2
Re: [PATCH] Check \0-termination of string in c_getstr
On Tue, Oct 11, 2016 at 11:27 AM, Martin Liška wrote: > As mentioned in the email that I reply to, c_getstr should check > null termination of string constants. > > Tests of the whole series have been running. Looks ok to me (if testing passes). Thanks, Richard. > Thanks, > Martin
[PATCH] Check \0-termination of string in c_getstr
As mentioned in the email that I reply to, c_getstr should check null termination of string constants. Tests of the whole series have been running. Thanks, Martin >From b446c659e839caa5ea5f36b06ec9110fe69f6e38 Mon Sep 17 00:00:00 2001 From: marxin Date: Mon, 10 Oct 2016 12:13:12 +0200 Subject: [PATCH 1/5] Check \0-termination of string in c_getstr gcc/ChangeLog: 2016-10-10 Martin Liska * fold-const.c (c_getstr): Guard string termination. --- gcc/fold-const.c | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/gcc/fold-const.c b/gcc/fold-const.c index 02aa484..a9e8650 100644 --- a/gcc/fold-const.c +++ b/gcc/fold-const.c @@ -14451,13 +14451,20 @@ c_getstr (tree src) if (src == 0) return 0; + unsigned HOST_WIDE_INT string_length = TREE_STRING_LENGTH (src) - 1; + const char *string = TREE_STRING_POINTER (src); + + /* If the string is not properly terminated, return 0. */ + if (string[string_length] != 0) +return 0; + if (offset_node == 0) -return TREE_STRING_POINTER (src); +return string; else if (!tree_fits_uhwi_p (offset_node) - || compare_tree_int (offset_node, TREE_STRING_LENGTH (src) - 1) > 0) + || compare_tree_int (offset_node, string_length) > 0) return 0; - return TREE_STRING_POINTER (src) + tree_to_uhwi (offset_node); + return string + tree_to_uhwi (offset_node); } #if CHECKING_P -- 2.9.2