This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
On Thu, 31 Jan 2013 15:23:07 +0100, Aleksandar Ristovski wrote:On 13-01-30 02:16 PM, Jan Kratochvil wrote:On Tue, 29 Jan 2013 17:15:13 +0100, Aleksandar Ristovski wrote:+ /* Section vma is unrelocated. If SO_BASE_ADDR is zero, then + use ASECT->VMA as-is. If not, then use offset + base addr. */ + res = target_verify_memory (data, (so_base_addr > 0)?
I do not see why to use target_verify_memory in this case.
While your comment below is correct, I find, since we introduced target_verify_memory already, this to be "more correct". Well, it is equivalent to what you are suggesting and I was considering doing simply read_memory/memcmp here, but then figured, target_verify_memory is more semantically correct.
target_verify_memory is there for large sections to compare only their 32-bit checksum. But build-id is already only 20 bytes long, with the protocol overhead the 4 vs. 20 bytes do not make a difference. And it needlessly weakens the check, it also does some patching of target_verify_memory. Just use target_read_memory and memcmp.
This is all true; however my opinion is that fallback in target_verify_memory is correct implementation as it allows using target_verify_memory where semantically suitable (like this place IMO) regardless of whether actual target implements it or not (e.g. core doesn't need to implement it; if it did, the implementation would probably be exactly the same as the fallback).
This is a bit nitpicking, primarily I do not see there much difference and we avoid dealing with target_verify_memory in this patch.
target_read_memory is already always implemented by the target.
With gdbserver <library-list-svr4/> protocol the build-id itself seems to me to be the natural way how to identify the library. While it could also send a 32-bit CRC in the XML protocol the build-id looks as more universal even for other possible GDB extensions in the future.
And thus optimizing local solib-svr4.c usage by 16 bytes saved by the 32-bit CRC seems off-topic to me, (1) it will work needlessly differently in both cases (vs. gdbserver) and (2) non-gdbserver usage is going to be deprecated so this is just a temporary code anyway.
Besides that target_verify_memory fallback should be put into default_verify_memory function and installed in to_verify_memory in all targets except that remote.c remote_verify_memory. target_* functions should be just dispatchers, not the implementations. (Yes, C++ would be easier.)
I find that l_addr_inferior
l_addr should be used, not l_addr_inferior. (Although one should not use rather either.)
remains to be 0 if prelinked object was not relocated. I haven't looked at gnu ld, but I would expect it's missing to set it correctly (this is misfortunate).
This behavior is correct. Changing it would break all the tools around.
l_addr just has wrong comment in glibc. I have pinged now the fix: [patch] Fix l_addr comment http://sourceware.org/ml/libc-alpha/2011-09/msg00151.html
iterate so->sections..so->sections_end which contains relocated ADDR (=target VMA). Then you can drop the svr4_unrelocated_vma and other calculations around.
Ok. I think this is what I tried first but then some testcases would fail. Will revisit.
There may be other issues but I am not aware of those.
--- Aleksandar
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |