On Wed, 2013-05-29 at 17:57 +0200, Jan Kratochvil wrote:
On Wed, 29 May 2013 11:50:26 +0200, Mark Wielaard wrote:
> On Tue, 2013-05-28 at 20:36 +0200, Jan Kratochvil wrote:
> > But when we talk about it in general, as a new patchset, I rather find a bug
> > dropping the path, when core file already contains the absolute pathname why
> > to hide it from the user?
>
> Right, if we can guarantee that path is the real one and not just an
> arbitrary guess.
It always was the real pathname - why it would not be? Or what have I forgot
about?
You forgot Nothing. I was just not very clear. I just meant that the
full path is there in eu-unstrip -n output if the actual file on the
file system is the one matching. It is now just missing in the output if
the file on the system is not a match. But that is fine.
> > + if (module_start <= module->l_ld &&
module->l_ld < module_end)
>
> Should the fixup not always be calculated and then the next check takes
> care of everything?
I do not find that safe; this could match unrelated L_LD from the other end of
address space just due to a "luck" it has the same '% PAGE_SIZE'
offset.
IIUC MODULE_START and MODULE_END are always safely located inside the segment
(considering they have BIAS already added); the segment address may be wrong
but it only matters there cannot be other shared library located at the same
address range. L_LD is also always safe as it comes from link map. Therefore
the first conditional is always safe:
if (module_start <= module->l_ld && module->l_ld < module_end)
I cannot much imagine how the second conditional after FIXUP adjustment
&& module_start + fixup <= module->l_ld
&& module->l_ld < module_end + fixup)
could fail but it is just a sanity check of FIXUP.
This is why I believe the first conditional should be there; the second
conditional may be redundant but it cannot hurt.
Or do you see some inconsistency in my safe / unsafe considerations above?
No, that seems logical. Thanks.
> > + GElf_Addr fixup = module->l_ld - dyn_vaddr;
> > + if ((fixup & (dwfl->segment_align - 1)) == 0
> > + && module_start + fixup <= module->l_ld
> > + && module->l_ld < module_end + fixup)
>
> Since the fixup could just be zero?
FIXUP will be in 99% of cases zero, what's the problem with it? I did not get
your comment here.
There is no problem. As you said above the extra check might be
redundant. But that is fine.
> Looks good to me. But please merge this commit and the code
movement
> commit into one, since those are really one change.
Yes, sure, it would break git bisect as it does not build with just the first
part applied.
Not checked in yet until the discussion gets finished.
I think it is finished now.
Thanks,
Mark