Re: SUSPICIOUS CODE

[Alan Modra <amodra@gmail.com>]

On Wed, Jul 12, 2023 at 05:12:13PM +0200, jacob navia wrote:
> 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? 

Take out the third loop and run the testsuite.  Does anything regress?

> 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,

That is exactly what needs to happen.

> 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.

Yes, I'm responsible for lots of suspicious code, but this isn't the
full history of get_frag_for_reloc.

> 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.

The code does not run through all relocations, just those created with
the .reloc directive.

> Thanks in advance for your help.
> 
> Jacob

-- 
Alan Modra
Australia Development Lab, IBM