Suspicious code

Suspicious code

[jacob navia]

[-- Attachment #1: Type: text/plain, Size: 2655 bytes --]

Consider this code:

1202 static fragS * get_frag_for_reloc (fragS *last_frag,
1203             const segment_info_type *seginfo,
1204             const struct reloc_list *r)
1205 {   
1206   fragS *f;
1207   
1208   for (f = last_frag; f != NULL; f = f->fr_next)
1209     if (f->fr_address <= r->u.b.r.address
1210     && r->u.b.r.address < f->fr_address + f->fr_fix)
1211       return f;
1212 
1213   for (f = seginfo->frchainP->frch_root; f != NULL; f = f->fr_next)
1214     if (f->fr_address <= r->u.b.r.address
1215     && r->u.b.r.address < f->fr_address + f->fr_fix)
1216       return f;
1217   
1218   for (f = seginfo->frchainP->frch_root; f != NULL; f = f->fr_next)
1219     if (f->fr_address <= r->u.b.r.address
1220     && r->u.b.r.address <= f->fr_address + f->fr_fix)
1221       return f;
1222 
1223   as_bad_where (r->file, r->line,
1224         _("reloc not within (fixed part of) section"));
1225   return NULL;
1226 }

This function consists of 3 loops: 1208-1211, 1213 to 1216 and 1218 to 1221. 

Lines 1213 - 1216 are ALMOST identical to lines 1218 to 1221. The ONLY difference that I can see is that the less in line 1215 is replaced by a less equal in line 1220.

But… why?

This code is searching the fragment that contains a given address in between the start and end addresses of the frags in question, either in the fragment list given by last_frag or in the list given by seginfo.

To know if a fragment is OK you should start with the given address and stop one memory address BEFORE the limit given by fr_address + f->fr_fix. That is what the first two loops are doing. The third loop repeats the second one and changes the less to less equal, so if fr_address+fr_fix is one MORE than the address it will still pass.

Why it is doing that? 

If that code is correct, it is obvious that we could merge the second and third loops and put a <= in t he second one and erase the third one… UNLESS priority should be given to matches that are less and not less equal, what seems incomprehensible … to me.

This change was introduced on Aug 18th 2011 by Mr Alan Modra with the rather terse comment: "(get_frag_for_reloc): New function. ». There are no further comments in the code at all.

This code is run after all relocations are fixed just before the software writes them out. The code is in file « write.c » in the gas directory. Note that this code runs through ALL relocations lists each time for EACH relocation, so it is quite expensive. In general the list data structure is not really optimal here but that is another story.

Thanks in advance for your help.

Jacob

Re: Suspicious code

[Andreas Schwab]

This looks like being part of gas, so you need to ask on
<binutils@sourceware.org>.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."