[Bug target/64003] valgrind complains about get_attr_length_nobnd in insn-attrtab.c from i386.md
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64003 Uroš Bizjak ubizjak at gmail dot com changed: What|Removed |Added CC||dcb314 at hotmail dot com --- Comment #29 from Uroš Bizjak ubizjak at gmail dot com --- *** Bug 66987 has been marked as a duplicate of this bug. ***
[Bug target/64003] valgrind complains about get_attr_length_nobnd in insn-attrtab.c from i386.md
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64003 Uroš Bizjak ubizjak at gmail dot com changed: What|Removed |Added Status|RESOLVED|REOPENED Last reconfirmed||2015-07-24 CC||ubizjak at gmail dot com Resolution|FIXED |--- Ever confirmed|0 |1 --- Comment #30 from Uroš Bizjak ubizjak at gmail dot com --- (In reply to Ilya Enkovich from comment #28) Fixed Let's fix this in the correct way, using ADJUST_INSN_LENGTH. We really don't want duplicated patterns.
[Bug target/64003] valgrind complains about get_attr_length_nobnd in insn-attrtab.c from i386.md
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64003 Uroš Bizjak ubizjak at gmail dot com changed: What|Removed |Added Status|REOPENED|ASSIGNED Assignee|enkovich.gnu at gmail dot com |ubizjak at gmail dot com --- Comment #31 from Uroš Bizjak ubizjak at gmail dot com --- Created attachment 36051 -- https://gcc.gnu.org/bugzilla/attachment.cgi?id=36051action=edit Patch that introduces ADJUST_INSN_LENGTH Patch that introduces ADJUST_INSN_LENGTH and tags relevant insns with maybe_prefix_bnd attribute.
[Bug target/64003] valgrind complains about get_attr_length_nobnd in insn-attrtab.c from i386.md
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64003 Uroš Bizjak ubizjak at gmail dot com changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED Target Milestone|--- |6.0 --- Comment #33 from Uroš Bizjak ubizjak at gmail dot com --- Fixed in mainline SVN by introducing ADJUST_INSN_LENGTH.
[Bug target/64003] valgrind complains about get_attr_length_nobnd in insn-attrtab.c from i386.md
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64003 --- Comment #32 from uros at gcc dot gnu.org --- Author: uros Date: Fri Jul 24 16:25:56 2015 New Revision: 226173 URL: https://gcc.gnu.org/viewcvs?rev=226173root=gccview=rev Log: PR target/64003 * config/i386/i386.h (ADJUST_INSN_LENGTH): New define. * config/i386/i386.md (maybe_prefix_bnd): New attribute. (*jcc_1, *jcc_2, jump, simple_return_internal) (simple_return_pop_internal): Set attribute maybe_prefix_bnd. Set length_nobnd attribute instead of length attribute. (indirect_jump, *tablejump_1): Set attribute maybe_prefix_bnd. (length_nobnd): Remove attribute. (length): Remove length_nobnd processing. Modified: trunk/gcc/ChangeLog trunk/gcc/config/i386/i386.h trunk/gcc/config/i386/i386.md
[Bug target/64003] valgrind complains about get_attr_length_nobnd in insn-attrtab.c from i386.md
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64003 Ilya Enkovich ienkovich at gcc dot gnu.org changed: What|Removed |Added Status|UNCONFIRMED |RESOLVED CC||ienkovich at gcc dot gnu.org Resolution|--- |FIXED --- Comment #28 from Ilya Enkovich ienkovich at gcc dot gnu.org --- Fixed
[Bug target/64003] valgrind complains about get_attr_length_nobnd in insn-attrtab.c from i386.md
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64003 --- Comment #21 from Ilya Enkovich enkovich.gnu at gmail dot com --- (In reply to Jeffrey A. Law from comment #20) Ilya, it's the function call in this code I think: (cond [(eq_attr length_nobnd !0) (plus (symbol_ref (ix86_bnd_prefixed_insn_p (insn))) (attr length_nobnd)) You're calling out to ix86_bnd_prefixed_insn_p, and that's problematical for branch shortening if I'm understanding Joern's comments here and David's comments in the PA port correctly. Then we have three problematic patterns and the easiest way to handle it is to get rid of ix86_bnd_prefixed_insn_p call in length computation for them. I think the easiest way to do it is to have separate bnd and nobnd patterns for these instructions. Attached patch helps me to resolve valgrind error. Is such approach fine?
[Bug target/64003] valgrind complains about get_attr_length_nobnd in insn-attrtab.c from i386.md
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64003 --- Comment #22 from Ilya Enkovich enkovich.gnu at gmail dot com --- Created attachment 34195 -- https://gcc.gnu.org/bugzilla/attachment.cgi?id=34195action=edit Proposed patch
[Bug target/64003] valgrind complains about get_attr_length_nobnd in insn-attrtab.c from i386.md
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64003 --- Comment #23 from Uroš Bizjak ubizjak at gmail dot com --- (In reply to Ilya Enkovich from comment #21) Then we have three problematic patterns and the easiest way to handle it is to get rid of ix86_bnd_prefixed_insn_p call in length computation for them. I think the easiest way to do it is to have separate bnd and nobnd patterns for these instructions. Attached patch helps me to resolve valgrind error. Is such approach fine? Maybe enabled attribute can help here to avoid unnecessary duplication.
[Bug target/64003] valgrind complains about get_attr_length_nobnd in insn-attrtab.c from i386.md
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64003 --- Comment #24 from Uroš Bizjak ubizjak at gmail dot com --- (In reply to Uroš Bizjak from comment #23) (In reply to Ilya Enkovich from comment #21) Then we have three problematic patterns and the easiest way to handle it is to get rid of ix86_bnd_prefixed_insn_p call in length computation for them. I think the easiest way to do it is to have separate bnd and nobnd patterns for these instructions. Attached patch helps me to resolve valgrind error. Is such approach fine? Maybe enabled attribute can help here to avoid unnecessary duplication. No, please disregard the above sentence. The aproach with two patterns looks OK AFAICS.
[Bug target/64003] valgrind complains about get_attr_length_nobnd in insn-attrtab.c from i386.md
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64003 rsandifo at gcc dot gnu.org rsandifo at gcc dot gnu.org changed: What|Removed |Added CC||rsandifo at gcc dot gnu.org --- Comment #25 from rsandifo at gcc dot gnu.org rsandifo at gcc dot gnu.org --- (In reply to Ilya Enkovich from comment #21) (In reply to Jeffrey A. Law from comment #20) Ilya, it's the function call in this code I think: (cond [(eq_attr length_nobnd !0) (plus (symbol_ref (ix86_bnd_prefixed_insn_p (insn))) (attr length_nobnd)) You're calling out to ix86_bnd_prefixed_insn_p, and that's problematical for branch shortening if I'm understanding Joern's comments here and David's comments in the PA port correctly. Then we have three problematic patterns and the easiest way to handle it is to get rid of ix86_bnd_prefixed_insn_p call in length computation for them. I think the easiest way to do it is to have separate bnd and nobnd patterns for these instructions. Attached patch helps me to resolve valgrind error. Is such approach fine? If all you want to do is add 1 byte to the length to account for a prefix then it might be cleaner to use ADJUST_INSN_LENGTH. You could then keep the single nobnd patterns.
[Bug target/64003] valgrind complains about get_attr_length_nobnd in insn-attrtab.c from i386.md
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64003 --- Comment #26 from Ilya Enkovich enkovich.gnu at gmail dot com --- (In reply to rsand...@gcc.gnu.org from comment #25) If all you want to do is add 1 byte to the length to account for a prefix then it might be cleaner to use ADJUST_INSN_LENGTH. You could then keep the single nobnd patterns. Currently i386 target doesn't have ADJUST_INSN_LENGTH defined. So I prefer to keep it so and have all length definitions explicit in md file.
[Bug target/64003] valgrind complains about get_attr_length_nobnd in insn-attrtab.c from i386.md
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64003 --- Comment #27 from ienkovich at gcc dot gnu.org --- Author: ienkovich Date: Fri Dec 5 16:00:52 2014 New Revision: 218426 URL: https://gcc.gnu.org/viewcvs?rev=218426root=gccview=rev Log: PR target/64003 * config/i386/i386.md (*jcc_1_bnd): New. (*jcc_2_bnd): New. (jump_bnd): New. (*jcc_1): Remove bnd prefix. (*jcc_2): Likewise. (jump): Likewise. Modified: trunk/gcc/ChangeLog trunk/gcc/config/i386/i386.md
[Bug target/64003] valgrind complains about get_attr_length_nobnd in insn-attrtab.c from i386.md
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64003
--- Comment #13 from Jorn Wolfgang Rennecke amylaar at gcc dot gnu.org ---
(In reply to David Malcolm from comment #6)
If I'm reading things right, this loop in shorten_branches populates
insn_lengths[uid] in order of the NEXT_INSN () iteration:
int (*length_fun) (rtx_insn *) = increasing ? insn_min_length :
insn_default_length;
for (insn_current_address = 0, insn = first;
insn != 0;
insn_current_address += insn_lengths[uid], insn = NEXT_INSN (insn))
{
uid = INSN_UID (insn);
insn_lengths[uid] = 0;
/* lots of logic, which can call length_fun, and hence
insn_min_length. */
}
and length_fun can call into insn_min_length, and hence this calls into
the get_attr_length_nobnd, which AIUI for this case is accessing lengths of
other insns before they've been populated: presumably for a jump forwards?
insn_min_length is not supposed to use current insn lengths.
genattrtab does not follow attributes for the purposes of determining
insn current length dependence.
So far we consider it the job of the port to provide
a length attribute that allows the calculation of minimum/maximum instruction
lengths with this limitation in mind.
That means the length attribute in i386.md is broken.
The get_attr_length_nobnd attribute need to be either inlined, or its use
guarded in a clause that appears to be length depepdent and supplies minimum
and maximum values.
AFAICS, the length attribute was broken in r217125
https://gcc.gnu.org/ml/gcc-cvs/2014-11/msg00133.html
[Bug target/64003] valgrind complains about get_attr_length_nobnd in insn-attrtab.c from i386.md
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64003 Jeffrey A. Law law at redhat dot com changed: What|Removed |Added Assignee|amylaar at gcc dot gnu.org |enkovich.gnu at gmail dot com --- Comment #14 from Jeffrey A. Law law at redhat dot com --- Damn Joern, I was looking at this comment in the PA port last night wondering if it was relevant to this discussion: ;; We use function calls to set the attribute length of calls and millicode ;; calls. This is necessary because of the large variety of call sequences. ;; Implementing the calculation in rtl is difficult as well as ugly. As ;; we need the same calculation in several places, maintenance becomes a ;; nightmare. ;; ;; However, this has a subtle impact on branch shortening. When the ;; expression used to set the length attribute of an instruction depends ;; on a relative address (e.g., pc or a branch address), genattrtab ;; notes that the insn's length is variable, and attempts to determine a ;; worst-case default length and code to compute an insn's current length. ;; The use of a function call hides the variable dependence of our calls ;; and millicode calls. The result is genattrtab doesn't treat the operation ;; as variable and it only generates code for the default case using our ;; function call. Is this documented anywhere? I certainly don't recall this restriction, but it does answer one of the questions I'd been kicking around in my head.
[Bug target/64003] valgrind complains about get_attr_length_nobnd in insn-attrtab.c from i386.md
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64003 --- Comment #15 from Jeffrey A. Law law at redhat dot com --- Just to be clear, that was a Damn as in Damn good find, sorry if it came out the wrong way.
[Bug target/64003] valgrind complains about get_attr_length_nobnd in insn-attrtab.c from i386.md
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64003 Jorn Wolfgang Rennecke amylaar at gcc dot gnu.org changed: What|Removed |Added CC||amylaar at gcc dot gnu.org --- Comment #16 from Jorn Wolfgang Rennecke amylaar at gcc dot gnu.org --- (In reply to Jeffrey A. Law from comment #14) Is this documented anywhere? I certainly don't recall this restriction, but it does answer one of the questions I'd been kicking around in my head. I've put a comment into sh.md to that effect - can't put a link to the gcc-cvs archive here because the code is from 1998, but here's an excerpt: ;; ??? This should use something like *branch_p (minus (match_dup 0) (pc)), ;; but getattrtab doesn't understand this. (define_attr length (cond [(eq_attr type cbranch) (cond [(eq_attr short_cbranch_p yes) (const_int 2) (eq_attr med_cbranch_p yes) (const_int 6) (eq_attr braf_cbranch_p yes) (const_int 12) ;; ??? using pc is not computed transitively. (ne (match_dup 0) (match_dup 0)) (const_int 14) ... The (ne (match_dup 0) (match_dup 0)) clause tells genattrtab that this cond form is length-varying. I had a patch to clear this up with a usable documented interface: https://gcc.gnu.org/ml/gcc-patches/2012-11/msg00473.html It got stuck in code review, so it's now a local patch in the Synopsys toolchains.
[Bug target/64003] valgrind complains about get_attr_length_nobnd in insn-attrtab.c from i386.md
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64003 --- Comment #17 from Ilya Enkovich enkovich.gnu at gmail dot com --- (In reply to Jorn Wolfgang Rennecke from comment #13) AFAICS, the length attribute was broken in r217125 https://gcc.gnu.org/ml/gcc-cvs/2014-11/msg00133.html If I understand the problem correctly the root is in attempt to get length of following instructions computing length for forwrad jump instruction. How comes r217125 is guilty for that? It doesn't introduce such computations, it just renames length attribute into length_nobnd for mentioned jump patterns. Do I miss something here?
[Bug target/64003] valgrind complains about get_attr_length_nobnd in insn-attrtab.c from i386.md
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64003 --- Comment #18 from Jorn Wolfgang Rennecke amylaar at gcc dot gnu.org --- (In reply to Ilya Enkovich from comment #17) If I understand the problem correctly the root is in attempt to get length of following instructions computing length for forwrad jump instruction. How comes r217125 is guilty for that? It doesn't introduce such computations, it just renames length attribute into length_nobnd for mentioned jump patterns. Do I miss something here? The length attribute is treated specially by genattrtab.
[Bug target/64003] valgrind complains about get_attr_length_nobnd in insn-attrtab.c from i386.md
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64003 --- Comment #19 from Jeffrey A. Law law at redhat dot com --- I was thinking more along the lines of documented in the texi documention for Length attributes.Useful to have in sh.md, but better documented in a location that is more likely to be read. I don't think that inlining ix86_bnd_prefixed_insn_p in the attribute stuff is really wise. Are you suggesting that we can put in a dummy clause similar to the sh or pa ports to resolve this issue? For reference the PA port's hack looks like: (set (attr length) (cond [(and (const_int 0) (eq (const_int 0) (pc))) (const_int 8)] (symbol_ref pa_attr_length_call (insn, 0]) The result of this hack is that during shortening these insns are considered as having a fixed length (8 bytes). So we might overestimte their actual size and generate more long branches than necessary. We have the length right by final, so the final version may be shorter than the conservative estimate during shorten-branches.
[Bug target/64003] valgrind complains about get_attr_length_nobnd in insn-attrtab.c from i386.md
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64003 --- Comment #20 from Jeffrey A. Law law at redhat dot com --- Ilya, it's the function call in this code I think: (cond [(eq_attr length_nobnd !0) (plus (symbol_ref (ix86_bnd_prefixed_insn_p (insn))) (attr length_nobnd)) You're calling out to ix86_bnd_prefixed_insn_p, and that's problematical for branch shortening if I'm understanding Joern's comments here and David's comments in the PA port correctly.
[Bug target/64003] valgrind complains about get_attr_length_nobnd in insn-attrtab.c from i386.md
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64003 --- Comment #8 from dmalcolm at gcc dot gnu.org --- Created attachment 34165 -- https://gcc.gnu.org/bugzilla/attachment.cgi?id=34165action=edit Patch to add instrumentation to final.c to track the reads/writes of insn_lengths The attached patch adds instrumentation of all reads/writes of the insn_lengths array.
[Bug target/64003] valgrind complains about get_attr_length_nobnd in insn-attrtab.c from i386.md
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64003 --- Comment #9 from dmalcolm at gcc dot gnu.org --- Created attachment 34166 -- https://gcc.gnu.org/bugzilla/attachment.cgi?id=34166action=edit Dump of RTL from the reproducer
[Bug target/64003] valgrind complains about get_attr_length_nobnd in insn-attrtab.c from i386.md
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64003 --- Comment #10 from dmalcolm at gcc dot gnu.org --- Created attachment 34167 -- https://gcc.gnu.org/bugzilla/attachment.cgi?id=34167action=edit Log from gdb session, with a conditional breakpoint to trap uninitialized reads This is a log of a gdb session debugging cc1 compiling the reproducer from comment #7, using the instrumentation patch from comment #8. I put a breakpoint on: fancy_element::operator int () const with condition that result == 0xabababab to handle reads of uninitialized elements from insn_lengths[]. I took a backtrace both times that the conditional breakpoint occurred.
[Bug target/64003] valgrind complains about get_attr_length_nobnd in insn-attrtab.c from i386.md
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64003
--- Comment #11 from dmalcolm at gcc dot gnu.org ---
Investigating the backtrace at the point of the uninit read shows that
the issue occurs in shorten_branches here:
#5 0x006f3263 in shorten_branches (first=0x71687700) at
../../src/gcc/final.c:1286
1284 else if (GET_CODE (body) != USE GET_CODE (body) != CLOBBER)
1285{
1286 insn_lengths[uid] = length_fun (insn);
1287 varying_length[uid] = insn_variable_length_p (insn);
1288}
(gdb) p uid
$7 = 10
(gdb) p length_fun
$8 = (int (*)(rtx_insn *)) 0xbeb5d1 insn_min_length(rtx_insn*)
(gdb) call debug(insn)
(jump_insn:TI 10 4 16 2 (set (pc)
(if_then_else (ne (reg:CCZ 17 flags)
(const_int 0 [0]))
(label_ref:DI 52)
(pc)))
/home/david/coding-3/gcc-git-jit-valgrind/get-attr-length-i386.c:3 610 {*jcc_1}
(expr_list:REG_DEAD (reg:CCZ 17 flags)
(int_list:REG_BR_PROB 3300 (nil)))
- 52)
I put insn-attrtab.c through GNU indent to make the conditional easier to read:
(gdb) down
#4 0x00beb7b7 in insn_min_length (insn=0x71685b40) at
insn-attrtab.c:276
276(ix86_bnd_prefixed_insn_p (insn)) + get_attr_length_nobnd (insn);
(gdb) list
271case 611:/* *jcc_2 */
272case 610:/* *jcc_1 */
273 extract_constrain_insn_cached (insn);
274 return
275/*line 516 ../../src/gcc/config/i386/i386.md*/
276(ix86_bnd_prefixed_insn_p (insn)) + get_attr_length_nobnd (insn);
277
278case 163:/* *truncxfdf2_mixed */
279case 162:/* *truncxfsf2_mixed */
280case 160:/* *truncdfsf_i387 */
(gdb) down
#3 0x00bf73b4 in get_attr_length_nobnd (insn=0x71685b40) at
insn-attrtab.c:19152
19152(insn_current_reference_address (insn))) = (-126))
(gdb) list
19147 if INSN_ADDRESSES_SET_P ()?
19148 INSN_ADDRESSES (INSN_UID
19149 (GET_CODE (operands[0]) ==
19150 LABEL_REF ? XEXP (operands[0],
191510) : operands[0])) : 0) -
19152(insn_current_reference_address (insn))) = (-126))
19153
19154 (((INSN_ADDRESSES_SET_P ()?
19155 INSN_ADDRESSES (INSN_UID
19156 (GET_CODE (operands[0]) ==
(gdb) down
#2 0x006f1e23 in insn_current_reference_address
(branch=0x71685b40) at ../../src/gcc/final.c:754
754 - align_fuzz (seq, dest, length_unit_log, ~0));
(gdb) list
749 BRANCH also has no INSN_SHUID. */
750 if (INSN_SHUID (seq) INSN_SHUID (dest))
751{
752 /* Forward branch. */
753 return (insn_last_address + insn_lengths[seq_uid]
754 - align_fuzz (seq, dest, length_unit_log, ~0));
755}
756 else
757{
758 /* Backward branch. */
(gdb) down
#1 0x006f1c4e in align_fuzz (start=0x71685b40, end=0x717b8900,
known_align_log=0, growth=4294967295)
at ../../src/gcc/final.c:703
703 align_addr = INSN_ADDRESSES (uid) - insn_lengths[uid];
(gdb) list
698 for (align_label = uid_align[uid]; align_label; align_label =
uid_align[uid])
699{
700 int align_addr, new_align;
701
702 uid = INSN_UID (align_label);
703 align_addr = INSN_ADDRESSES (uid) - insn_lengths[uid];
704 if (uid_shuid[uid] end_shuid)
705break;
706 known_align_log = LABEL_TO_ALIGNMENT (align_label);
707 new_align = 1 known_align_log;
i.e. the uninitialized read occurs within align_fuzz in insn_lengths[uid] in
this expressions:
703 align_addr = INSN_ADDRESSES (uid) - insn_lengths[uid];
(gdb) p uid
$10 = 52
handling a forward branch within a *jcc_2 insn.
Running valgrind with vgdb to get the precise location of its
warnings indicates they are here within get_attr_length_nobnd in
insn-attrtab.c:19152:
19147 if INSN_ADDRESSES_SET_P ()?
19148 INSN_ADDRESSES (INSN_UID
19149 (GET_CODE (operands[0]) ==
19150 LABEL_REF ? XEXP (operands[0],
191510) : operands[0])) : 0) -
19152(insn_current_reference_address (insn))) = (-126))
19153
19154 (((INSN_ADDRESSES_SET_P ()?
19155 INSN_ADDRESSES (INSN_UID
19156 (GET_CODE (operands[0]) ==
i.e. at the logical-AND at line 19153.
Valgrind presumably is noticing the uninitialized trait of this
read, then propagating it through to the result of align_fuzz,
and thence to insn_current_reference_address, and hence to the whole
of the first argument of the logical-AND.
Hence the decision about whether to process the second argument of
the logical-AND is a jump that relies on an uninitialized value, and
hence valgrind complains.
[Bug target/64003] valgrind complains about get_attr_length_nobnd in insn-attrtab.c from i386.md
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64003 --- Comment #12 from dmalcolm at gcc dot gnu.org --- (In reply to dmalcolm from comment #11) Running valgrind with vgdb to get the precise location of its warnings indicates they are here within get_attr_length_nobnd in insn-attrtab.c:19152: 19147 if INSN_ADDRESSES_SET_P ()? 19148 INSN_ADDRESSES (INSN_UID 19149 (GET_CODE (operands[0]) == 19150 LABEL_REF ? XEXP (operands[0], 19151 0) : operands[0])) : 0) - 19152 (insn_current_reference_address (insn))) = (-126)) 19153 19154 (((INSN_ADDRESSES_SET_P ()? 19155 INSN_ADDRESSES (INSN_UID 19156 (GET_CODE (operands[0]) == i.e. at the logical-AND at line 19153. Valgrind presumably is noticing the uninitialized trait of this read, then propagating it through to the result of align_fuzz, and thence to insn_current_reference_address, and hence to the whole of the first argument of the logical-AND. Hence the decision about whether to process the second argument of the logical-AND is a jump that relies on an uninitialized value, and hence valgrind complains. i.e. the issue is that evaluating both sides of the (and) expression at line 10931 in: 10920 (define_insn *jcc_1 10921[(set (pc) 10922 (if_then_else (match_operator 1 ix86_comparison_operator 10923[(reg FLAGS_REG) (const_int 0)]) 10924(label_ref (match_operand 0)) 10925(pc)))] 10926 10927%!%+j%C1\t%l0 10928[(set_attr type ibr) 10929 (set_attr modrm 0) 10930 (set (attr length_nobnd) 10931 (if_then_else (and (ge (minus (match_dup 0) (pc)) 10932(const_int -126)) 10933(lt (minus (match_dup 0) (pc)) 10934(const_int 128))) 10935 (const_int 2) 10936 (const_int 6)))]) 10937 for a forward jump, leads to reads of uninitialized items from insn_lengths in align_fuzz for the uid for the jump target. Hence we have an (and (UNINITIALIZED_1) (WILL_BE_UNINITIALIZED_2)) and hence the decision about whether to short-circuit the read of WILL_BE_UNINITIALIZED_2 is a conditional jump that depends on UNINITIALIZED_1.
[Bug target/64003] valgrind complains about get_attr_length_nobnd in insn-attrtab.c from i386.md
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64003
--- Comment #7 from dmalcolm at gcc dot gnu.org ---
A more minimal reproducer:
$ cat get-attr-length-i386.c
int test_pr64003 (int p)
{
if (p == 0)
return 5;
if (p == 1)
return 7;
return 0;
}
$ valgrind ./cc1 get-attr-length-i386.c -O2
[Bug target/64003] valgrind complains about get_attr_length_nobnd in insn-attrtab.c from i386.md
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64003
--- Comment #6 from dmalcolm at gcc dot gnu.org ---
If I'm reading things right, this loop in shorten_branches populates
insn_lengths[uid] in order of the NEXT_INSN () iteration:
int (*length_fun) (rtx_insn *) = increasing ? insn_min_length :
insn_default_length;
for (insn_current_address = 0, insn = first;
insn != 0;
insn_current_address += insn_lengths[uid], insn = NEXT_INSN (insn))
{
uid = INSN_UID (insn);
insn_lengths[uid] = 0;
/* lots of logic, which can call length_fun, and hence insn_min_length.
*/
}
and length_fun can call into insn_min_length, and hence this calls into the
get_attr_length_nobnd, which AIUI for this case is accessing lengths of other
insns before they've been populated: presumably for a jump forwards?
FWIW this untested patch silences the valgrind warning:
diff --git a/gcc/final.c b/gcc/final.c
index c3805c9..0805418 100644
--- a/gcc/final.c
+++ b/gcc/final.c
@@ -1019,7 +1019,7 @@ shorten_branches (rtx_insn *first)
return;
/* Allocate the rest of the arrays. */
- insn_lengths = XNEWVEC (int, max_uid);
+ insn_lengths = XCNEWVEC (int, max_uid);
insn_lengths_max_uid = max_uid;
/* Syntax errors can lead to labels being outside of the main insn stream.
Initialize insn_addresses, so that we get reproducible results. */
@@ -1127,8 +1127,6 @@ shorten_branches (rtx_insn *first)
{
uid = INSN_UID (insn);
- insn_lengths[uid] = 0;
-
if (LABEL_P (insn))
{
int log = LABEL_TO_ALIGNMENT (insn);
