optc-save-gen.awk: use uintrptr_t for casting of a pointer (PR bootstrap/90543).

Hi.

The patch is about using of uintptr_t instead unsigned long that's
being used for printing value of a pointer.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin

gcc/ChangeLog:

2019-05-22  Martin Liska  <mliska@suse.cz>

	PR bootstrap/90543
	* optc-save-gen.awk: Use uintptr_t instead of
	unsigned long.
---
 gcc/optc-save-gen.awk | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

On Wed, May 22, 2019 at 09:39:52AM +0200, Martin Liška wrote:
> The patch is about using of uintptr_t instead unsigned long that's
> being used for printing value of a pointer.
> 
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> 
> Ready to be installed?

No.  You haven't adjusted the fprintf format strings for this change,
so it will break build on any target where uintptr_t is not unsigned long.

I don't see any gcc/ code using the PRIxPTR and similar macros, so no idea
about the portability of that, but I'd think we just should keep that as is.

	Jakub

On 5/22/19 9:50 AM, Jakub Jelinek wrote:
> On Wed, May 22, 2019 at 09:39:52AM +0200, Martin Liška wrote:
>> The patch is about using of uintptr_t instead unsigned long that's
>> being used for printing value of a pointer.
>>
>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>
>> Ready to be installed?
> 
> No.  You haven't adjusted the fprintf format strings for this change,
> so it will break build on any target where uintptr_t is not unsigned long.
> 
> I don't see any gcc/ code using the PRIxPTR and similar macros, so no idea
> about the portability of that, but I'd think we just should keep that as is.

Ok.

Martin

> 
> 	Jakub
>

On Wed, May 22, 2019 at 10:07:33AM +0200, Martin Liška wrote:
> On 5/22/19 9:50 AM, Jakub Jelinek wrote:
> > On Wed, May 22, 2019 at 09:39:52AM +0200, Martin Liška wrote:
> >> The patch is about using of uintptr_t instead unsigned long that's
> >> being used for printing value of a pointer.
> >>
> >> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> >>
> >> Ready to be installed?
> > 
> > No.  You haven't adjusted the fprintf format strings for this change,
> > so it will break build on any target where uintptr_t is not unsigned long.
> > 
> > I don't see any gcc/ code using the PRIxPTR and similar macros, so no idea
> > about the portability of that, but I'd think we just should keep that as is.
> 
> Ok.

If the problem is just warnings, perhaps you could do
(unsigned long) (uintptr_t) casts instead of directly to (unsigned long),
ideally only for the options with pointer types.
Seems we have:
                else if (otype ~ "^const char \\**$")
                        var_opt_string[n_opt_string++] = name;
                else
                        var_opt_other[n_opt_other++] = name;
and so the
print "  fputs (\"\\n\", file);";
for (i = 0; i < n_opt_other; i++) {
        print "  if (ptr1->x_" var_opt_other[i] " != ptr2->x_" var_opt_other[i] ")";
        print "    fprintf (file, \"%*s%s (%#lx/%#lx)\\n\",";
        print "             indent_to, \"\",";
        print "             \"" var_opt_other[i] "\",";
        print "             (unsigned long)ptr1->x_" var_opt_other[i] ",";
        print "             (unsigned long)ptr2->x_" var_opt_other[i] ");";
        print "";
}
printing isn't done for strings, we use in that case
for (i = 0; i < n_opt_string; i++) {
        name = var_opt_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_to, \"\",";
        print "             \"" name "\",";
        print "             ptr1->x_" name ",";
        print "             ptr2->x_" name ");";
        print "";
}
instead.  So, the only problem is in:
print "  fputs (\"\\n\", file);";
for (i = 0; i < n_target_other; i++) {
        print "  if (ptr->x_" var_target_other[i] ")";
        hwi = host_wide_int[var_target_other[i]]
        if (hwi == "yes")
                print "    fprintf (file, \"%*s%s (%#\" HOST_WIDE_INT_PRINT \"x)\\n\",";
        else
                print "    fprintf (file, \"%*s%s (%#lx)\\n\",";
        print "             indent, \"\",";
        print "             \"" var_target_other[i] "\",";
        if (hwi == "yes")
                print "             ptr->x_" var_target_other[i] ");";
        else
                print "             (unsigned long)ptr->x_" var_target_other[i] ");";
        print "";
}
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).

	Jakub

On 5/22/19 11:40 AM, Jakub Jelinek wrote:
> On Wed, May 22, 2019 at 10:07:33AM +0200, Martin Liška wrote:
>> On 5/22/19 9:50 AM, Jakub Jelinek wrote:
>>> On Wed, May 22, 2019 at 09:39:52AM +0200, Martin Liška wrote:
>>>> The patch is about using of uintptr_t instead unsigned long that's
>>>> being used for printing value of a pointer.
>>>>
>>>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>>>
>>>> Ready to be installed?
>>>
>>> No.  You haven't adjusted the fprintf format strings for this change,
>>> so it will break build on any target where uintptr_t is not unsigned long.
>>>
>>> I don't see any gcc/ code using the PRIxPTR and similar macros, so no idea
>>> about the portability of that, but I'd think we just should keep that as is.
>>
>> Ok.
> 
> If the problem is just warnings, perhaps you could do
> (unsigned long) (uintptr_t) casts instead of directly to (unsigned long),
> ideally only for the options with pointer types.

I'm sending patch for it.

> Seems we have:
>                 else if (otype ~ "^const char \\**$")
>                         var_opt_string[n_opt_string++] = name;
>                 else
>                         var_opt_other[n_opt_other++] = name;
> and so the
> print "  fputs (\"\\n\", file);";
> for (i = 0; i < n_opt_other; i++) {
>         print "  if (ptr1->x_" var_opt_other[i] " != ptr2->x_" var_opt_other[i] ")";
>         print "    fprintf (file, \"%*s%s (%#lx/%#lx)\\n\",";
>         print "             indent_to, \"\",";
>         print "             \"" var_opt_other[i] "\",";
>         print "             (unsigned long)ptr1->x_" var_opt_other[i] ",";
>         print "             (unsigned long)ptr2->x_" var_opt_other[i] ");";
>         print "";
> }
> printing isn't done for strings, we use in that case
> for (i = 0; i < n_opt_string; i++) {
>         name = var_opt_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_to, \"\",";
>         print "             \"" name "\",";
>         print "             ptr1->x_" name ",";
>         print "             ptr2->x_" name ");";
>         print "";
> }
> instead.  So, the only problem is in:
> print "  fputs (\"\\n\", file);";
> for (i = 0; i < n_target_other; i++) {
>         print "  if (ptr->x_" var_target_other[i] ")";
>         hwi = host_wide_int[var_target_other[i]]
>         if (hwi == "yes")
>                 print "    fprintf (file, \"%*s%s (%#\" HOST_WIDE_INT_PRINT \"x)\\n\",";
>         else
>                 print "    fprintf (file, \"%*s%s (%#lx)\\n\",";
>         print "             indent, \"\",";
>         print "             \"" var_target_other[i] "\",";
>         if (hwi == "yes")
>                 print "             ptr->x_" var_target_other[i] ");";
>         else
>                 print "             (unsigned long)ptr->x_" var_target_other[i] ");";
>         print "";
> }
> 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.

Martin

> 
> 	Jakub
>

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  <jakub@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)\");";
 	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)\");";
+	print "";
+}
+
 print "}";
 
 print "";


	Jakub
--- options-save.c.jj	2019-04-15 19:47:25.093447074 +0200
+++ options-save.c	2019-05-22 13:16:32.036436037 +0200
@@ -2141,25 +2141,25 @@ cl_optimization_print (FILE *file,
              "flag_mlow_precision_sqrt",
              ptr->x_flag_mlow_precision_sqrt);
 
-  if (ptr->x_optimize)
+  if (ptr->x_str_align_functions)
     fprintf (file, "%*s%s (%s)\n",
              indent_to, "",
              "str_align_functions",
              ptr->x_str_align_functions);
 
-  if (ptr->x_optimize_size)
+  if (ptr->x_str_align_jumps)
     fprintf (file, "%*s%s (%s)\n",
              indent_to, "",
              "str_align_jumps",
              ptr->x_str_align_jumps);
 
-  if (ptr->x_optimize_debug)
+  if (ptr->x_str_align_labels)
     fprintf (file, "%*s%s (%s)\n",
              indent_to, "",
              "str_align_labels",
              ptr->x_str_align_labels);
 
-  if (ptr->x_optimize_fast)
+  if (ptr->x_str_align_loops)
     fprintf (file, "%*s%s (%s)\n",
              indent_to, "",
              "str_align_loops",
@@ -3812,8 +3812,8 @@ cl_optimization_print_diff (FILE *file,
     fprintf (file, "%*s%s (%s/%s)\n",
              indent_to, "",
              "str_align_functions",
-             ptr1->x_str_align_functions,
-             ptr2->x_str_align_functions);
+             ptr1->x_str_align_functions ? : ptr1->x_str_align_functions : "(null)",
+             ptr2->x_str_align_functions ? : ptr1->x_str_align_functions : "(null)");
 
   if (ptr1->x_str_align_jumps != ptr2->x_str_align_jumps
       || (!ptr1->x_str_align_jumps || !ptr2->x_str_align_jumps
@@ -3821,8 +3821,8 @@ cl_optimization_print_diff (FILE *file,
     fprintf (file, "%*s%s (%s/%s)\n",
              indent_to, "",
              "str_align_jumps",
-             ptr1->x_str_align_jumps,
-             ptr2->x_str_align_jumps);
+             ptr1->x_str_align_jumps ? : ptr1->x_str_align_jumps : "(null)",
+             ptr2->x_str_align_jumps ? : ptr1->x_str_align_jumps : "(null)");
 
   if (ptr1->x_str_align_labels != ptr2->x_str_align_labels
       || (!ptr1->x_str_align_labels || !ptr2->x_str_align_labels
@@ -3830,8 +3830,8 @@ cl_optimization_print_diff (FILE *file,
     fprintf (file, "%*s%s (%s/%s)\n",
              indent_to, "",
              "str_align_labels",
-             ptr1->x_str_align_labels,
-             ptr2->x_str_align_labels);
+             ptr1->x_str_align_labels ? : ptr1->x_str_align_labels : "(null)",
+             ptr2->x_str_align_labels ? : ptr1->x_str_align_labels : "(null)");
 
   if (ptr1->x_str_align_loops != ptr2->x_str_align_loops
       || (!ptr1->x_str_align_loops || !ptr2->x_str_align_loops
@@ -3839,8 +3839,8 @@ cl_optimization_print_diff (FILE *file,
     fprintf (file, "%*s%s (%s/%s)\n",
              indent_to, "",
              "str_align_loops",
-             ptr1->x_str_align_loops,
-             ptr2->x_str_align_loops);
+             ptr1->x_str_align_loops ? : ptr1->x_str_align_loops : "(null)",
+             ptr2->x_str_align_loops ? : ptr1->x_str_align_loops : "(null)");
 
 }
 
@@ -3861,7 +3861,6 @@ cl_target_option_save (struct cl_target_
   ptr->x_aarch64_stack_protector_guard_offset = opts->x_aarch64_stack_protector_guard_offset;
   ptr->x_aarch64_enable_bti = opts->x_aarch64_enable_bti;
   ptr->x_aarch64_isa_flags = opts->x_aarch64_isa_flags;
-  ptr->x_aarch64_branch_protection_string = opts->x_aarch64_branch_protection_string;
   ptr->x_target_flags = opts->x_target_flags;
   ptr->x_aarch64_cmodel_var = opts->x_aarch64_cmodel_var;
   ptr->x_aarch64_ra_sign_scope = opts->x_aarch64_ra_sign_scope;
@@ -3870,6 +3869,7 @@ cl_target_option_save (struct cl_target_
   ptr->x_aarch64_fix_a53_err843419 = opts->x_aarch64_fix_a53_err843419;
   ptr->x_flag_omit_leaf_frame_pointer = opts->x_flag_omit_leaf_frame_pointer;
   ptr->x_pcrelative_literal_loads = opts->x_pcrelative_literal_loads;
+  ptr->x_aarch64_branch_protection_string = opts->x_aarch64_branch_protection_string;
 }
 
 /* Restore selected current options from a structure.  */
@@ -3881,7 +3881,6 @@ cl_target_option_restore (struct gcc_opt
   opts->x_aarch64_stack_protector_guard_offset = ptr->x_aarch64_stack_protector_guard_offset;
   opts->x_aarch64_enable_bti = ptr->x_aarch64_enable_bti;
   opts->x_aarch64_isa_flags = ptr->x_aarch64_isa_flags;
-  opts->x_aarch64_branch_protection_string = ptr->x_aarch64_branch_protection_string;
   opts->x_target_flags = ptr->x_target_flags;
   opts->x_aarch64_cmodel_var = ptr->x_aarch64_cmodel_var;
   opts->x_aarch64_ra_sign_scope = ptr->x_aarch64_ra_sign_scope;
@@ -3890,6 +3889,7 @@ cl_target_option_restore (struct gcc_opt
   opts->x_aarch64_fix_a53_err843419 = ptr->x_aarch64_fix_a53_err843419;
   opts->x_flag_omit_leaf_frame_pointer = ptr->x_flag_omit_leaf_frame_pointer;
   opts->x_pcrelative_literal_loads = ptr->x_pcrelative_literal_loads;
+  opts->x_aarch64_branch_protection_string = ptr->x_aarch64_branch_protection_string;
 
   if (targetm.target_option.restore)
     targetm.target_option.restore (opts, ptr);
@@ -3902,12 +3902,6 @@ cl_target_option_print (FILE *file,
                         struct cl_target_option *ptr)
 {
   fputs ("\n", file);
-  if (ptr->x_aarch64_branch_protection_string)
-    fprintf (file, "%*s%s (%#lx)\n",
-             indent, "",
-             "aarch64_branch_protection_string",
-             (unsigned long)ptr->x_aarch64_branch_protection_string);
-
   if (ptr->x_target_flags)
     fprintf (file, "%*s%s (%#lx)\n",
              indent, "",
@@ -3956,6 +3950,12 @@ cl_target_option_print (FILE *file,
              "pcrelative_literal_loads",
              ptr->x_pcrelative_literal_loads);
 
+  if (ptr->x_aarch64_branch_protection_string)
+    fprintf (file, "%*s%s (%s)\n",
+             indent, "",
+             "aarch64_branch_protection_string",
+             ptr->x_aarch64_branch_protection_string);
+
 
   if (targetm.target_option.print)
     targetm.target_option.print (file, indent, ptr);
@@ -3969,13 +3969,6 @@ cl_target_option_print_diff (FILE *file,
                              struct cl_target_option *ptr2 ATTRIBUTE_UNUSED)
 {
   fputs ("\n", file);
-  if (ptr1->x_aarch64_branch_protection_string != ptr2->x_aarch64_branch_protection_string)
-    fprintf (file, "%*s%s (%#lx/%#lx)\n",
-             indent, "",
-             "aarch64_branch_protection_string",
-             (unsigned long)ptr1->x_aarch64_branch_protection_string,
-             (unsigned long)ptr2->x_aarch64_branch_protection_string);
-
   if (ptr1->x_target_flags != ptr2->x_target_flags)
     fprintf (file, "%*s%s (%#lx/%#lx)\n",
              indent, "",
@@ -4032,6 +4025,15 @@ cl_target_option_print_diff (FILE *file,
              ptr1->x_pcrelative_literal_loads,
              ptr2->x_pcrelative_literal_loads);
 
+  if (ptr1->x_aarch64_branch_protection_string != ptr2->x_aarch64_branch_protection_string
+      || (!ptr1->x_aarch64_branch_protection_string || !ptr2->x_aarch64_branch_protection_string
+          || strcmp (ptr1->x_aarch64_branch_protection_string, ptr2->x_aarch64_branch_protection_string)))
+    fprintf (file, "%*s%s (%s/%s)\n",
+             indent, "",
+             "aarch64_branch_protection_string",
+             ptr1->x_aarch64_branch_protection_string ? : ptr1->x_aarch64_branch_protection_string : "(null)",
+             ptr2->x_aarch64_branch_protection_string ? : ptr1->x_aarch64_branch_protection_string : "(null)");
+
 }
 
 /* Compare two target options  */

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  <jakub@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
>