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 _______________________________________________ elfutils-devel mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/elfutils-devel
