On Wed, May 22, 2019 at 01:27:04PM +0200, Jakub Jelinek wrote:
> 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?
I ended up sending a non-final version which actually didn't compile (extra
: after ?), but also found another issue since then, conditions like:
if (ptr1->x_str_align_functions != ptr2->x_str_align_functions
|| (!ptr1->x_str_align_functions || !ptr2->x_str_align_functions
|| strcmp (ptr1->x_str_align_functions, ptr2->x_str_align_functions)))
fprintf (file, "%*s%s (%s/%s)\n",
indent_to, "",
"str_align_functions",
ptr1->x_str_align_functions,
ptr2->x_str_align_functions);
don't really make any sense, if ptr1->x_str_align_functions !=
ptr2->x_str_align_functions
is false, then either both are NULL (but then we do not want to print
anything), or strcmp will be necessarily 0 (for both pointers). What we
actually want is
if (ptr1->x_str_align_functions != ptr2->x_str_align_functions
&& (!ptr1->x_str_align_functions || !ptr2->x_str_align_functions
|| strcmp (ptr1->x_str_align_functions, ptr2->x_str_align_functions)))
note the && instead of ||.
So, here is a new patch and a new aarch64 options-save.c diff.
Ok for trunk if it passes full bootstrap/regtest on x86_64-linux (passed
already normal build on aarch64-linux cross)?
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 and use && instead of first || in the
guarding condition. 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:33:11.128183956 +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] "\",";
@@ -326,13 +326,13 @@ for (i = 0; i < n_opt_char; i++) {
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 " && (!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 " 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:33:38.032746240 +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",
@@ -3807,40 +3807,40 @@ cl_optimization_print_diff (FILE *file,
ptr2->x_flag_mlow_precision_sqrt);
if (ptr1->x_str_align_functions != ptr2->x_str_align_functions
- || (!ptr1->x_str_align_functions || !ptr2->x_str_align_functions
+ && (!ptr1->x_str_align_functions || !ptr2->x_str_align_functions
|| strcmp (ptr1->x_str_align_functions,
ptr2->x_str_align_functions)))
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
+ && (!ptr1->x_str_align_jumps || !ptr2->x_str_align_jumps
|| strcmp (ptr1->x_str_align_jumps, ptr2->x_str_align_jumps)))
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
+ && (!ptr1->x_str_align_labels || !ptr2->x_str_align_labels
|| strcmp (ptr1->x_str_align_labels, ptr2->x_str_align_labels)))
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
+ && (!ptr1->x_str_align_loops || !ptr2->x_str_align_loops
|| strcmp (ptr1->x_str_align_loops, ptr2->x_str_align_loops)))
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 */
