* Ensure correct symbol-file when attaching to a (remote) process @ 2012-12-21 6:05 Raphael Zulliger 2012-12-21 16:11 ` Jan Kratochvil 0 siblings, 1 reply; 16+ messages in thread From: Raphael Zulliger @ 2012-12-21 6:05 UTC (permalink / raw) To: gdb Hi, I'm searching for a GDB feature that ensures that a symbol-file matches the running process to which I attach. More precisely, my use-case is that I attach to an already running embedded-system (bare metal) by extended-remote. Unfortunately, having a wrong ELF may result in showing wrong call stacks, etc. In worst case, you wont even notice that somethings wrong. I wasn't able to find something like that in the doc nor have I ever seen something like that in the remote serial protocol definition. Is there really no such mechanism in GDB? Raphael ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Ensure correct symbol-file when attaching to a (remote) process 2012-12-21 6:05 Ensure correct symbol-file when attaching to a (remote) process Raphael Zulliger @ 2012-12-21 16:11 ` Jan Kratochvil 2012-12-21 18:17 ` John Gilmore 2012-12-21 21:12 ` Aleksandar Ristovski 0 siblings, 2 replies; 16+ messages in thread From: Jan Kratochvil @ 2012-12-21 16:11 UTC (permalink / raw) To: Raphael Zulliger; +Cc: gdb On Fri, 21 Dec 2012 07:05:37 +0100, Raphael Zulliger wrote: > Is there really no such mechanism in GDB? There is no such reliable mechanism in general. One could verify build-id in the target, probably that the build-id note is at the same VMA as present in the local symbol file. But then also many binaries/compilers do not provide build-id by default (ld option --build-id). Jan ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Ensure correct symbol-file when attaching to a (remote) process 2012-12-21 16:11 ` Jan Kratochvil @ 2012-12-21 18:17 ` John Gilmore 2013-01-02 17:57 ` Pedro Alves 2012-12-21 21:12 ` Aleksandar Ristovski 1 sibling, 1 reply; 16+ messages in thread From: John Gilmore @ 2012-12-21 18:17 UTC (permalink / raw) To: Jan Kratochvil; +Cc: Raphael Zulliger, gdb > > Is there really no such mechanism in GDB? > > There is no such reliable mechanism in general. If gdb had a local copy of the binary object file (not just the symbols), it could try to read arbitrary locations in that file, and read the same locations in the remote target, and compare the two. (This would take a bit of work, since it would have to manually walk the target stack to find and access the local object file.) Only if you read back every byte of the object file could you be SURE that the remote target contained the same thing. In that case you might as well have just written the entire object file TO the target. Either would be extremely slow. Testing just a few places in memory is unlikely to detect the kinds of small changes that debugging cycles frequently make to programs. This is why gdb doesn't do it by default (I considered it when making the target stack first work). If someone wrote such a command as a GDB enhancement, and submitted the patch, it could probably be added as a specific command ("verify-target percent" where percent is what fraction of the address space to compare -- default should probably be super quick, 0.00001% or something). You'd also have to make a decision about what to do with data areas and bss (zeroed) areas. In a remote target, after the program has been run one or more times, these will normally be overwritten with updated variable values. When restarting the program, there is no guarantee that the program's initialization code will restore them, nor that the rest of the program's code won't depend on the initial values in these variables. When merely connecting GDB to debug an already-running target in mid-execution, these variables can have any values. The best you can do in an automated way is to check the areas of memory that are intended to contain instructions and read-only data. (And in some targets, such as the Linux kernel, even that check will fail, since it links initialization code separately from running code, and overwrites the initialization code with data areas after it has initialized the kernel.) John ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Ensure correct symbol-file when attaching to a (remote) process 2012-12-21 18:17 ` John Gilmore @ 2013-01-02 17:57 ` Pedro Alves 2013-01-12 10:59 ` Martin Runge ` (2 more replies) 0 siblings, 3 replies; 16+ messages in thread From: Pedro Alves @ 2013-01-02 17:57 UTC (permalink / raw) To: John Gilmore; +Cc: Jan Kratochvil, Raphael Zulliger, gdb On 12/21/2012 07:17 PM, John Gilmore wrote: > The best you can do in an automated way is to check the areas > of memory that are intended to contain instructions and read-only > data. For bare metal targets, that's often good enough, and GDB does have support that built in: (gdb) help compare-sections Compare section data on target to the exec file. Argument is a single section name (default: all loaded sections). A build id check would really be ideal. -- Pedro Alves ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Ensure correct symbol-file when attaching to a (remote) process 2013-01-02 17:57 ` Pedro Alves @ 2013-01-12 10:59 ` Martin Runge [not found] ` <50EA78FB.3040609@indel.ch> 2013-01-29 3:18 ` Raphael Zulliger 2 siblings, 0 replies; 16+ messages in thread From: Martin Runge @ 2013-01-12 10:59 UTC (permalink / raw) To: Pedro Alves, gdb; +Cc: John Gilmore, Jan Kratochvil, Raphael Zulliger I would really like to see such a feature in gdb, too. Our projects are Linux based, but the problem remains the same. We tried a self made patch that compared the most important sections of host and target binaries as a whole. Our projects are quite large (~260 MB stripped binaries) and that patch caused extra 30-60 seconds startup time for debugging, so we removed it again. in gdb 7.3 I see a similar feature already build in. I see warnings like this: warning: the debug information found in "/lib/libname.so" does not match "/lib/libname.so" (CRC mismatch). It shows up for all DSOs, that do not match between host and target, e.g. if the solib search path is not set correctly and gdb on the host looks at the host's library instead of the one that matches the target's. Who does this compare to your patch? 2013/1/2 Pedro Alves <palves@redhat.com>: > On 12/21/2012 07:17 PM, John Gilmore wrote: >> The best you can do in an automated way is to check the areas >> of memory that are intended to contain instructions and read-only >> data. > > For bare metal targets, that's often good enough, and GDB does have > support that built in: > > (gdb) help compare-sections > Compare section data on target to the exec file. > Argument is a single section name (default: all loaded sections). > > A build id check would really be ideal. > > -- > Pedro Alves > ^ permalink raw reply [flat|nested] 16+ messages in thread
[parent not found: <50EA78FB.3040609@indel.ch>]
* Re: Ensure correct symbol-file when attaching to a (remote) process [not found] ` <50EA78FB.3040609@indel.ch> @ 2013-01-14 18:57 ` Pedro Alves 0 siblings, 0 replies; 16+ messages in thread From: Pedro Alves @ 2013-01-14 18:57 UTC (permalink / raw) To: Raphael Zulliger, gdb (adding back gdb@, which seems to have been removed from CC by mistake). On 01/07/2013 07:27 AM, Raphael Zulliger wrote: > On 01/02/2013 06:56 PM, Pedro Alves wrote: >> A build id check would really be ideal. > I agree. (--build-id is a very interesting feature which I was not > aware of. Thanks for that hint Jan) > > In our scenario, our GDB stub could get that build-id from the > running target: Our embedded systems provides a variable read/write > mechanism accessible by the stub. Moreover, the embedded system > could be made aware of the address of the build-id by introducing > variables around the .note.gnu.build-id section in the linker > script. Assuming the enough sticks around in the binary that goes to the system to have the stub find those variables without relying on GDB, sounds like something that should work. > > Therefore, if the GDB remote serial protocol would offer a way to > "get" that id from the stub and GDB would offer a feature to > compare/check that id, it'd perfectly work for us. AFAIK there are > no "generic" ways of transferring data by the remote serial > protocol, therefore, we'd need to extend it accordingly, right? For getting the build-id blob, it sounds pretty much like something for qXfer:$object:read. You'd just need to define a new (e.g.) "gnu-build-id" $object (there's some mechanics involved, but also examples to follow). > Could such a protocol extension, and the according build-id > comparison check make it into GDB? IMO, yes. For GNU/Linux and other systems, we'd need a build-id per executable/library/module, so it seems like the build-id could/should be reported in the existing qXfer:libraries/qXfer:libraries-svr4 as well. -- Pedro Alves ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Ensure correct symbol-file when attaching to a (remote) process 2013-01-02 17:57 ` Pedro Alves 2013-01-12 10:59 ` Martin Runge [not found] ` <50EA78FB.3040609@indel.ch> @ 2013-01-29 3:18 ` Raphael Zulliger 2013-01-29 3:51 ` Raphael Zulliger 2 siblings, 1 reply; 16+ messages in thread From: Raphael Zulliger @ 2013-01-29 3:18 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb (I apologize: I've replied to this mail some weeks ago - but forgot to CC to the list. I therefore resend this email) On 01/02/2013 06:56 PM, Pedro Alves wrote: > A build id check would really be ideal. I agree. (--build-id is a very interesting feature which I was not aware of. Thanks for that hint Jan) In our scenario, our GDB stub could get that build-id from the running target: Our embedded systems provides a variable read/write mechanism accessible by the stub. Moreover, the embedded system could be made aware of the address of the build-id by introducing variables around the .note.gnu.build-id section in the linker script. Therefore, if the GDB remote serial protocol would offer a way to "get" that id from the stub and GDB would offer a feature to compare/check that id, it'd perfectly work for us. AFAIK there are no "generic" ways of transferring data by the remote serial protocol, therefore, we'd need to extend it accordingly, right? Could such a protocol extension, and the according build-id comparison check make it into GDB? ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Ensure correct symbol-file when attaching to a (remote) process 2013-01-29 3:18 ` Raphael Zulliger @ 2013-01-29 3:51 ` Raphael Zulliger 2013-02-06 18:47 ` Tom Tromey 0 siblings, 1 reply; 16+ messages in thread From: Raphael Zulliger @ 2013-01-29 3:51 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb On 01/29/2013 04:18 AM, Raphael Zulliger wrote: > I agree. > (--build-id is a very interesting feature which I was not aware of. > Thanks for that hint Jan) > > In our scenario, our GDB stub could get that build-id from the running > target: Our embedded systems provides a variable read/write mechanism > accessible by the stub. Moreover, the embedded system could be made > aware of the address of the build-id by introducing variables around > the .note.gnu.build-id section in the linker script. > Here's the solution we came up with. Unfortunately, the solution is not very GDB-like mainly because the lack of time to create high quality GDB patches. But at least I want to share our (low-end) solution with you. (Note: I'm definitely not saying that this is the ideal solution! It's just good enough for our use cases.) I post this message because of two purposes: 1. As a reference if someone is searching the web 2. To check if someone comes up with a better solution for reading the build-id out from an elf file Here's what we've done in order to read the build-id from within our C/C++ code of our embedded software: - We pass "-Wl,--build-id" to the linker invocation - We extended the linker script by: .note : { PROVIDE (__NOTE_BUILDID_BEGIN__ = .); *(.note.gnu.build-id) PROVIDE (__NOTE_BUILDID_END__ = .); } > ram - In the C/C++ code we have something like this: extern unsigned int __NOTE_BUILDID_BEGIN__; extern unsigned int __NOTE_BUILDID_END__; With these two variables at hand, we are able to propagate the build-id to our GDB-stub by our own "PC to embedded system communication". - On the GDB side, we hacked some python code to read the build-id out from the ELF file: def GetBuildId(): BuildId = "" # we only ever have 1 ELF file (or 0 in case of a user error): for file in gdb.objfiles(): cmd = 'objdump -s -j .note ' + file.filename p = subprocess.Popen(cmd, shell=True, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.STDOUT) output = p.stdout.read() for line in output.splitlines(): if len(line) > 0 and line[0] == ' ': for entry in line.split()[1:5]: BuildId = BuildId + entry return BuildId This is definitely no high-end code, but it does its job. Among others, It's potentially unstable if objdump changes it's output format... I searched the web for a better way of getting the content of a certain section out from an object file, but couldn't find a solution. If someone knows about a GDB/Python interface for doing so, I would definitely like to hear about it! The build-id read out from the ELF file will then be sent to our GDB stub in a non-GDB way (again some kind of home-made communication, which we'd already in use). Our GDB stub finally compares the build-id read from the ELF file with the one read from the embedded system and can compare them for equality. Raphael ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Ensure correct symbol-file when attaching to a (remote) process 2013-01-29 3:51 ` Raphael Zulliger @ 2013-02-06 18:47 ` Tom Tromey 0 siblings, 0 replies; 16+ messages in thread From: Tom Tromey @ 2013-02-06 18:47 UTC (permalink / raw) To: Raphael Zulliger; +Cc: Pedro Alves, gdb >>>>> "Raphael" == Raphael Zulliger <zulliger@indel.ch> writes: Raphael> I searched the web for a better way of getting the content Raphael> of a certain section out from an object file, but couldn't find a Raphael> solution. If someone knows about a GDB/Python interface for doing so, Raphael> I would definitely like to hear about it! I've been meaning to add this for ages but it has never made it to the top of my list. I finally filed a bug for it: http://sourceware.org/bugzilla/show_bug.cgi?id=15108 Tom ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Ensure correct symbol-file when attaching to a (remote) process 2012-12-21 16:11 ` Jan Kratochvil 2012-12-21 18:17 ` John Gilmore @ 2012-12-21 21:12 ` Aleksandar Ristovski 2012-12-22 0:38 ` John Gilmore 2013-01-29 3:18 ` Raphael Zulliger 1 sibling, 2 replies; 16+ messages in thread From: Aleksandar Ristovski @ 2012-12-21 21:12 UTC (permalink / raw) To: Jan Kratochvil; +Cc: Raphael Zulliger, gdb On 12-12-21 11:11 AM, Jan Kratochvil wrote: > On Fri, 21 Dec 2012 07:05:37 +0100, Raphael Zulliger wrote: >> Is there really no such mechanism in GDB? > > There is no such reliable mechanism in general. > > One could verify build-id in the target, probably that the build-id note is at > the same VMA as present in the local symbol file. > > But then also many binaries/compilers do not provide build-id by default (ld > option --build-id). Interesting timing. I have just posted http://sourceware.org/ml/gdb-patches/2012-12/msg00776.html addressing this issue. The check is not exhaustive, but for most practical purposes it should suffice. It verifies that in-memory elf header and pheaders match those found in the bfd. Of course it will not detect difference in all cases, e.g. very small changes that do not affect loadable segment size. Example: - static int foo; - static int bar; + static int bar; + static int foo; Still, it should be much better than no check at all. --- Aleksandar ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Ensure correct symbol-file when attaching to a (remote) process 2012-12-21 21:12 ` Aleksandar Ristovski @ 2012-12-22 0:38 ` John Gilmore 2012-12-22 2:54 ` Aleksandar Ristovski 2013-01-29 3:18 ` Raphael Zulliger 1 sibling, 1 reply; 16+ messages in thread From: John Gilmore @ 2012-12-22 0:38 UTC (permalink / raw) To: Aleksandar Ristovski; +Cc: Jan Kratochvil, Raphael Zulliger, gdb > Interesting timing. I have just posted > http://sourceware.org/ml/gdb-patches/2012-12/msg00776.html addressing > this issue. That's a useful patch. It's smart to read the headers from the "symbol_file" bfd (well, in this case the solib bfd used for symbols) rather than from the "exec_file" bfd. But doesn't it read the ELF header twice? In svr4_validate_hdrs_match, it reads it once in the call to read_elf_header_from_target, and then reads it again by calling read_program_headers_from_target which again calls read_elf_header_from_target. I suspect that you should pass the already-read ELF header into each of the read_program_headers functions. Also, in read_elf_header_from_bfd, BFD has probably already read the ELF header, when it first opened the file, and has it lying around. This code should not be (1) reading it again, nor (2) seeking to file offset "0" (not even a #define, not even "0L", but an int constant!) to get it. Also, it might be simpler in read_program_headers_from_target to just translate the ELF header into its internal BFD struct form - then all that manual messing with byte orders and field lengths could be eliminated. See how read_program_headers_from_bfd does it. You could also just call bfd_get_elf_phdrs (and before that, bfd_get_elf_phdr_upper_bound to size the buffer) which would avoid spreading details into GDB about these headers and how they are read and translated into local byte order. BFD even has a bfd_elf_bfd_from_remote_memory function for creating a BFD by portably reading the target's ELF header and phdrs out of memory. Also, is "validate" the right name for this? That works for checking shared libraries, but it's not as obvious what this function should do when e.g. checking the argument to symbol_file, or immediately after doing "target attach remote". If "validation" fails, should we refuse to read that symbol file, or refuse to attach to the remote target? What exactly are we validating? Perhaps "compare_headers" or "do_symbols_and_target_match" are better names. With those things addressed, it's a good start. Then, shouldn't this call be implemented for non-shared-library symbol files (like Mr. Zulliger was hoping for), and for object file formats other than ELF? Has this code been tested when the target uses a different byte order than the host? Probably not, since it is only currently invoked for shared library loads, which usually only happen native. Is there a test case written for it? Could it be easily extended to check a build-id when one exists? Or would that require different function arguments, or need to happen at a different time? Should "target attach" do this kind of validation traffic to and from the target? Or should checking this be a separate step initiated by the user? The target might be in a relatively sensitive state -- or the user might not want to wait for this check -- immediately after attaching. We could add yet another GDB setting for automatic or manual validation, defaulting to "check", and requiring those who want to bypass the check to set that setting. Is that too much complication for the gain? John ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Ensure correct symbol-file when attaching to a (remote) process 2012-12-22 0:38 ` John Gilmore @ 2012-12-22 2:54 ` Aleksandar Ristovski 2012-12-24 20:02 ` Aleksandar Ristovski 0 siblings, 1 reply; 16+ messages in thread From: Aleksandar Ristovski @ 2012-12-22 2:54 UTC (permalink / raw) To: John Gilmore; +Cc: Jan Kratochvil, Raphael Zulliger, gdb On 12-12-21 08:38 PM, John Gilmore wrote: >> Interesting timing. I have just posted >> http://sourceware.org/ml/gdb-patches/2012-12/msg00776.html addressing >> this issue. > > That's a useful patch. It's smart to read the headers from the > "symbol_file" bfd (well, in this case the solib bfd used for symbols) > rather than from the "exec_file" bfd. > > But doesn't it read the ELF header twice? In > svr4_validate_hdrs_match, it reads it once in the call to > read_elf_header_from_target, and then reads it again by calling > read_program_headers_from_target which again calls > read_elf_header_from_target. I suspect that you should pass the > already-read ELF header into each of the read_program_headers > functions. Yes, it reads it twice, but our target memory could (and probably should) cache this. Note that we are talking about sizeof(Elf32_Ehdr) or sizeof(Elf64_Ehdr) which is not a lot. The way it is proposed in the patch matches corresponding read_*_from_bfd functions which do not require passing elf header to read phdrs. This is not to say my proposal is the only way of doing it, what you are suggesting is valid approach as well. However, I would prefer to leave it as it is in the patch. > > Also, in read_elf_header_from_bfd, BFD has probably already read the > ELF header, when it first opened the file, and has it lying around. > This code should not be (1) reading it again, nor (2) seeking to file > offset "0" (not even a #define, not even "0L", but an int constant!) > to get it. Elf header is always at 0, so I'm not sure what define would you prefer seeing there instead? Elf header is specifically located at offset 0, this is by the System V spec. of the elf format. I can put 0L but in this case, does it really matter? > > Also, it might be simpler in read_program_headers_from_target to just > translate the ELF header into its internal BFD struct form - then all > that manual messing with byte orders and field lengths could be > eliminated. See how read_program_headers_from_bfd does it. Note that this is because we read directly from target memory. > > You could also just call bfd_get_elf_phdrs (and before that, > bfd_get_elf_phdr_upper_bound to size the buffer) which would avoid > spreading details into GDB about these headers and how they are read > and translated into local byte order. BFD even has a > bfd_elf_bfd_from_remote_memory function for creating a BFD by portably > reading the target's ELF header and phdrs out of memory. I will have to look at how could bfd be used to read from target memory, but the idea is to compare 'raw' data. There is no need to translate into internal form - in-memory data needs to match that from the file, no cross-endiannes happens here at all (except for, of course, getting phdr offset directly from in-memory elf header; I intentionally did not want to use bfd's internal representation for that as it corresponds to the header read from the file residing on the host, not necessarily matching file, which is what we are checking here. > > Also, is "validate" the right name for this? That works for checking > shared libraries, but it's not as obvious what this function should > do when e.g. checking the argument to symbol_file, or immediately > after doing "target attach remote". If "validation" fails, should > we refuse to read that symbol file, or refuse to attach to the remote > target? What exactly are we validating? Perhaps "compare_headers" > or "do_symbols_and_target_match" are better names. I am not a native English speaker and as such I will accept better proposals, but I thought 'validate' would be the right name. We are at the point where we are reading symbol file. Symbol file is generic here (and 'symbol' somewhat may confuse. The file we found on the host could be completely stripped and so only "minimal symbols" (those that can be derived from .dynsym/.dynstr) may be available, but that is irrelevant to what is being validated here), we are really opening a file on the host that corresponds to loaded object in target memory. Therefore, attach is only one way of getting into this situation. It may be provoked by e.g. noshared ... shared. Regardless of how did we get here, we found a file that gdb thinks corresponds to that in the target's memory. We are now validating and answering the question: "Is the found file valid?". I chose 'validate' as a generic concept, someone may decide to implement some other type of validation, e.g. bit-for-bit of text segment, someone might decide to open file using target fileio and read it all. My proposal is to have check that is practical: it is almost always sufficient and a lot faster than doing full segments (i.e. exhaustive) comparison. Full segment comparison approach may not include anything that could have been modified by the dynamic linker, so we would be restricted to full text compare (so still not exhaustive, but slightly better than what is in the proposal). In effect, bfd corresponding to the object that does not pass validation gets thrown away and we do not read symbols from it. We had done some work on it such as relocating sections, and general bfd work but that's about it. This is all host work so I don't think it affects performance much. If this is a concern, validation could take place earlier, but I don't see a need at the moment. > > With those things addressed, it's a good start. Then, shouldn't this > call be implemented for non-shared-library symbol files (like > Mr. Zulliger was hoping for), and for object file formats other than > ELF? Not for non-elf formats, but the API is there. It can be implemented (I did not plan to do that). I'm not sure I understand the first part of the question: it doesn't matter whether the object is shared or not. What matters is that it is an object loaded by the dynamic linker (therefore this validation is performed only for dynamic processes that link typically shared objects). I believe we already have executable comparison in place, but I will double check. > > Has this code been tested when the target uses a different byte > order than the host? Probably not, since it is only currently invoked > for shared library loads, which usually only happen native. Is > there a test case written for it? This was implemented primarily for remote scenarios as library mismatch is a lot more likely in this use-case. I believe I addressed different endian-es where applicable. I also put comment on why is bit comparison of 'raw' headers fine. We are reading library that resides on the host, but is built for the target. Therefore, endianes in the headers must match and there is no need to translate it. Testcase: I haven't written one yet. > > Could it be easily extended to check a build-id when one exists? Or > would that require different function arguments, or need to happen at > a different time? This is stronger than build-id check. Note also that this does not presume that any section header (nor section headers in general) be present in the loaded image. build-id is not prescribed to be mapped to a loadable segment and can not be counted on to be present in targets memory. I specifically did not want to require any file operations on the target. Notice that data read from the target relies _only_ on data that _must_ be present for dynamic linker, and consequently must reside in target memory. It does not attempt to use section headers (e.g. we strip them for some of our libraries) nor bits that are optional. > > Should "target attach" do this kind of validation traffic to and from > the target? Or should checking this be a separate step initiated by > the user? The target might be in a relatively sensitive state -- or > the user might not want to wait for this check -- immediately after > attaching. We could add yet another GDB setting for automatic or > manual validation, defaulting to "check", and requiring those who want > to bypass the check to set that setting. Is that too much > complication for the gain? Traffic should be fairly small. Elf64_Ehdr is 64 bytes. Phdrs may vary as their number varies, but from my observations today (on native gnu/linux 64 bit) gnulibc has phdrs that are less than 1K in size. This is one time work, so I think it won't be detrimental to performance, while providing very useful functionality (those who have to deal with library mismatches know what I am talking about). As stated above, this check happens on shared library load. My example from the patch is just an illustration of a real scenario (but admittedly less common) that can happen when debugging natively. I do not think there is a need for another option; for example, we perform "separate symbol file" CRC calculation without an option to disable it (and this *could* be time consuming). > > John > Thanks for looking at the patch and your feedback, Aleksandar ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Ensure correct symbol-file when attaching to a (remote) process 2012-12-22 2:54 ` Aleksandar Ristovski @ 2012-12-24 20:02 ` Aleksandar Ristovski 2012-12-26 0:47 ` John Gilmore 0 siblings, 1 reply; 16+ messages in thread From: Aleksandar Ristovski @ 2012-12-24 20:02 UTC (permalink / raw) Cc: John Gilmore, Jan Kratochvil, Raphael Zulliger, gdb Hello John, On 12-12-21 09:54 PM, Aleksandar Ristovski wrote: > On 12-12-21 08:38 PM, John Gilmore wrote: >> >> With those things addressed, it's a good start. I have addressed some of your concerns, and some issues I found while writing the testcase. Please take a look: http://sourceware.org/ml/gdb-patches/2012-12/msg00809.html Thank you, Aleksandar ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Ensure correct symbol-file when attaching to a (remote) process 2012-12-24 20:02 ` Aleksandar Ristovski @ 2012-12-26 0:47 ` John Gilmore 2012-12-27 20:13 ` Aleksandar Ristovski 0 siblings, 1 reply; 16+ messages in thread From: John Gilmore @ 2012-12-26 0:47 UTC (permalink / raw) To: Aleksandar Ristovski; +Cc: John Gilmore, Jan Kratochvil, Raphael Zulliger, gdb > I have addressed some of your concerns, and some issues I found while > writing the testcase. Please take a look: > http://sourceware.org/ml/gdb-patches/2012-12/msg00809.html Thank you! There are several places where you assume that there are only two possible pointer sizes ("if (ptr_size == 4)" and "else"). These should abort -- or return false -- if they encounter a pointer size they don't understand. And what is this constant "4" doing in there? In read_program_headers_from_target, you initialize load_base_trusted to 0, then in the first executable statement, you test it and abort if it's zero. It took me some detailed reading to discover that in another initializer, you pass load_base_trusted's address to lm_addr_check, which modifies it as a side effect. Could you move that call to lm_addr_check to just before the if statement, so it's clearly in a place where people can see that a side effect will occur? In svr4_validate_ehdr_match, you compare the "magic numbers" by directly feeding struct pointers to memcmp. You should compare the magic number field the same way you compare all the other fields -- as e.g. ehdr1->_32.e_magic. I'm still bothered that you read the symbol file's ELF header three times, once when BFD opens the file, once when you read the ELF header because you don't use BFD's copy, and once again when you need it to find the phdrs. And you're still reading it from location "0" in the file. Ditto for accessing the target memory twice, unnecessarily. I am also wondering why all this infrastructure has been made specific to shared libraries (it's in the solib ops vector). The original request on the mailing list was for this validation to occur for ordinary symbol-files; this implementation prevents it from ever being used for that. John ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Ensure correct symbol-file when attaching to a (remote) process 2012-12-26 0:47 ` John Gilmore @ 2012-12-27 20:13 ` Aleksandar Ristovski 0 siblings, 0 replies; 16+ messages in thread From: Aleksandar Ristovski @ 2012-12-27 20:13 UTC (permalink / raw) To: John Gilmore; +Cc: gdb On 12-12-25 08:35 PM, John Gilmore wrote: >> I have addressed some of your concerns, and some issues I found while >> writing the testcase. Please take a look: >> http://sourceware.org/ml/gdb-patches/2012-12/msg00809.html > > Thank you! > > There are several places where you assume that there are only > two possible pointer sizes ("if (ptr_size == 4)" and "else"). > These should abort -- or return false -- if they encounter a > pointer size they don't understand. And what is this constant "4" > doing in there? > > In read_program_headers_from_target, you initialize load_base_trusted > to 0, then in the first executable statement, you test it and abort if > it's zero. It took me some detailed reading to discover that in > another initializer, you pass load_base_trusted's address to > lm_addr_check, which modifies it as a side effect. Could you move that > call to lm_addr_check to just before the if statement, so it's clearly > in a place where people can see that a side effect will occur? > > In svr4_validate_ehdr_match, you compare the "magic numbers" by > directly feeding struct pointers to memcmp. You should compare > the magic number field the same way you compare all the other > fields -- as e.g. ehdr1->_32.e_magic. > > I'm still bothered that you read the symbol file's ELF header three > times, once when BFD opens the file, once when you read the ELF header > because you don't use BFD's copy, and once again when you need it to > find the phdrs. And you're still reading it from location "0" in the > file. Ditto for accessing the target memory twice, unnecessarily. > > I am also wondering why all this infrastructure has been made specific > to shared libraries (it's in the solib ops vector). The original > request on the mailing list was for this validation to occur for > ordinary symbol-files; this implementation prevents it from ever > being used for that. > > John Thank you for your comments. I will try to address your concerns and come up with a better patch. Due to other priorities, it will most probably not be before another few weeks. --- Aleksandar ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Ensure correct symbol-file when attaching to a (remote) process 2012-12-21 21:12 ` Aleksandar Ristovski 2012-12-22 0:38 ` John Gilmore @ 2013-01-29 3:18 ` Raphael Zulliger 1 sibling, 0 replies; 16+ messages in thread From: Raphael Zulliger @ 2013-01-29 3:18 UTC (permalink / raw) To: Aleksandar Ristovski; +Cc: gdb (I apologize: I've replied to this mail some weeks ago - but forgot to CC to the list. I therefore resend this email) On 12/21/2012 10:11 PM, Aleksandar Ristovski wrote: > Interesting timing. I have just posted > http://sourceware.org/ml/gdb-patches/2012-12/msg00776.html addressing > this issue. Indeed . Thanks! > > The check is not exhaustive, but for most practical purposes it should > suffice. It verifies that in-memory elf header and pheaders match > those found in the bfd. > > Of course it will not detect difference in all cases, e.g. very small > changes that do not affect loadable segment size. Example: > > - static int foo; > - static int bar; > + static int bar; > + static int foo; > > Still, it should be much better than no check at all. I'm not very familiar with ELF header information... But after to some quick investigation, it seems that this mechanism "only" helps to detect changes that affect "changes in size" or "changes of address", right? Moreover, if I understood it correctly, we'd have a fundamental problem using this mechanism in our scenario: We don't download the ELF file to the embedded system. Instead, we process the ELF file with objcopy (-Osrec) on the developer system and only download the produced SREC file - which contains no ELF header information anymore. ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2013-02-06 18:47 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-12-21 6:05 Ensure correct symbol-file when attaching to a (remote) process Raphael Zulliger 2012-12-21 16:11 ` Jan Kratochvil 2012-12-21 18:17 ` John Gilmore 2013-01-02 17:57 ` Pedro Alves 2013-01-12 10:59 ` Martin Runge [not found] ` <50EA78FB.3040609@indel.ch> 2013-01-14 18:57 ` Pedro Alves 2013-01-29 3:18 ` Raphael Zulliger 2013-01-29 3:51 ` Raphael Zulliger 2013-02-06 18:47 ` Tom Tromey 2012-12-21 21:12 ` Aleksandar Ristovski 2012-12-22 0:38 ` John Gilmore 2012-12-22 2:54 ` Aleksandar Ristovski 2012-12-24 20:02 ` Aleksandar Ristovski 2012-12-26 0:47 ` John Gilmore 2012-12-27 20:13 ` Aleksandar Ristovski 2013-01-29 3:18 ` Raphael Zulliger
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).