On 5/22/19 1:27 PM, Jakub Jelinek wrote:
> On Wed, May 22, 2019 at 01:19:37PM +0200, Martin Liška wrote:
>>> and another similar block with ptr1/ptr2 later. So, I wonder if you just
>>> shouldn't follow what is done for var_opt_string vs. var_opt_other even in
>>> the var_target_other case, do:
>>> + else if (otype ~ "^const char \\**$")
>>> + var_target_string[n_target_string++] = name;
>>> else
>>> var_target_other[n_target_other++] = name;
>>> + initialization etc. and handle var_target_string differently from
>>> var_target_other for the printing (print it actually with %s instead of
>>> %#lx).
>>
>> No, you want to print a value, which can be an integer type, or a pointer
>> to an array. So you don't want to use %s.
>
> No, IMHO you do want to use %s, patch below. I've found a couple of other
> bugs in optc-save-gen.awk. Attached is a diff between aarch64 target
> options-save.c before and after this patch.
>
> The other bugs were that cl_optimization_print had a typo in the condition
> and so guarded the printing on the value of an unrelated option, and that
> not all C libraries print (null) instead of crashing when passing NULL to
> %s.
>
> Tested on x86_64-linux and aarch64-linux (cross), ok for trunk?
>
> 2019-05-22 Jakub Jelinek <ja...@redhat.com>
>
> PR bootstrap/90543
> * optc-save-gen.awk: In cl_optimization_print, use correct condition
> for var_opt_string printing. In cl_optimization_print_diff, print
> (null) instead of invoking undefined behavior if one of the
> var_opt_string pointers is NULL. For var_target_other options, handle
> const char * target variables similarly to const char * optimize node
> variables.
>
> --- gcc/optc-save-gen.awk.jj 2019-02-15 08:16:22.838993512 +0100
> +++ gcc/optc-save-gen.awk 2019-05-22 13:16:26.596524297 +0200
> @@ -253,7 +253,7 @@ for (i = 0; i < n_opt_char; i++) {
> }
>
> for (i = 0; i < n_opt_string; i++) {
> - print " if (ptr->x_" var_opt_char[i] ")";
> + print " if (ptr->x_" var_opt_string[i] ")";
> print " fprintf (file, \"%*s%s (%s)\\n\",";
> print " indent_to, \"\",";
> print " \"" var_opt_string[i] "\",";
> @@ -331,8 +331,8 @@ for (i = 0; i < n_opt_string; i++) {
> print " fprintf (file, \"%*s%s (%s/%s)\\n\",";
> print " indent_to, \"\",";
> print " \"" name "\",";
> - print " ptr1->x_" name ",";
> - print " ptr2->x_" name ");";
> + print " ptr1->x_" name " ? : ptr1->x_" name " :
> \"(null)\",";
> + print " ptr2->x_" name " ? : ptr1->x_" name " :
> \"(null)\");";
There are typos: "? :"
options-save.c:3786:67: error: expected ‘)’ before ‘:’ token
ptr2->x_str_align_labels ? : ptr1->x_str_align_labels : "(null)");
^~
> print "";
> }
>
> @@ -349,6 +349,7 @@ n_target_char = 0;
> n_target_short = 0;
> n_target_int = 0;
> n_target_enum = 0;
> +n_target_string = 0;
> n_target_other = 0;
>
> if (have_save) {
> @@ -381,6 +382,8 @@ if (have_save) {
> if (otype == var_type(flags[i]))
> var_target_range[name] = ""
> }
> + else if (otype ~ "^const char \\**$")
> + var_target_string[n_target_string++] = name;
> else
> var_target_other[n_target_other++] = name;
> }
> @@ -429,6 +432,10 @@ for (i = 0; i < n_target_char; i++) {
> print " ptr->x_" var_target_char[i] " = opts->x_" var_target_char[i]
> ";";
> }
>
> +for (i = 0; i < n_target_string; i++) {
> + print " ptr->x_" var_target_string[i] " = opts->x_"
> var_target_string[i] ";";
> +}
> +
> print "}";
>
> print "";
> @@ -461,6 +468,10 @@ for (i = 0; i < n_target_char; i++) {
> print " opts->x_" var_target_char[i] " = ptr->x_" var_target_char[i]
> ";";
> }
>
> +for (i = 0; i < n_target_string; i++) {
> + print " opts->x_" var_target_string[i] " = ptr->x_"
> var_target_string[i] ";";
> +}
> +
> # This must occur after the normal variables in case the code depends on
> those
> # variables.
> print "";
> @@ -530,6 +541,15 @@ for (i = 0; i < n_target_char; i++) {
> print "";
> }
>
> +for (i = 0; i < n_target_string; i++) {
> + print " if (ptr->x_" var_target_string[i] ")";
> + print " fprintf (file, \"%*s%s (%s)\\n\",";
> + print " indent, \"\",";
> + print " \"" var_target_string[i] "\",";
> + print " ptr->x_" var_target_string[i] ");";
> + print "";
> +}
> +
> print "";
> print " if (targetm.target_option.print)";
> print " targetm.target_option.print (file, indent, ptr);";
> @@ -605,6 +625,19 @@ for (i = 0; i < n_target_char; i++) {
> print "";
> }
>
> +for (i = 0; i < n_target_string; i++) {
> + name = var_target_string[i]
> + print " if (ptr1->x_" name " != ptr2->x_" name "";
> + print " || (!ptr1->x_" name" || !ptr2->x_" name
> + print " || strcmp (ptr1->x_" name", ptr2->x_" name ")))";
> + print " fprintf (file, \"%*s%s (%s/%s)\\n\",";
> + print " indent, \"\",";
> + print " \"" name "\",";
> + print " ptr1->x_" name " ? : ptr1->x_" name " :
> \"(null)\",";
> + print " ptr2->x_" name " ? : ptr1->x_" name " :
> \"(null)\");";
Likewise here.
Martin
> + print "";
> +}
> +
> print "}";
>
> print "";
>
>
> Jakub
>
