On Tue, Feb 08, 2022 at 10:22:06AM +0530, Vimal Agrawal wrote: > > > Actually I had it under (!best) in my first patch. I had to change it > > > because it was resolving the address to symbols like __this_module for > > > init address at times which is not correct for text address. > > > > Can you say that again? > > I hit this a few times later after the first patch. Basically there > are all symbols in symbol table and best could be none zero ( means it > matched some symbol) but it may not be match to .text symbol for text > address ( esp. when --strip-unneeded is used as there are very few > symbols left after stripping) You are saying that sometimes having "best" evaluated to non zero yields incorrect results, where the symbol found is actualy not a .text symbol for a text address? If so, is this really true for cases where no stripping is used? If so this is bigger news and I'd like to address this separately in another commit but we need proof, not just speculation. And you seem to be suggesting that this seems to hold more true when "--strip-unneeded" is used given there are fewer symbols left after striping? Did I comprehend what you are trying to say correctly? > > OK so you're saying sometimes "best" is not true when we use > > INSTALL_MOD_STRIP="--strip-unneeded"? This is news. > > > yes, best can be non zero and may not resolve to .text address when > --strip-unneeded is used. OK. > without stripping, it will definitely > resolve to some .text address closely matching in case of no stripping OK so there is no issue when stripping is used. > but it can go wrong with stripping. I have seen it a few times post > the first patch during testing. OK then we need to take care your added heuristics do not affect non-stripping. > > In particulr you seem to be suggesting that if --strip-unneeded was > > used "best" could be incorrect for !is_module_text_address(). > > > best could be incorrect even for text address when --strip-unneeded is used. > e.g. in my case, it is resolving .init.text address to __this_module You should be explicit about this in your commit log. > > In any case, you completely changed things in your patch and did not > > mention *any* of this in your follow up patch, leaving me to question > > the validity of all this work. > > > The Only change I did from the first patch was to move this hunk out of (!best). > Yes, I missed commenting it in the code. When you submit a v2 patch and you change something like that you must clarify changes which are not clear either in the commit log or below the --- lines after the diffstat and before the actual patch. Each new patch iteration should have a set of bullets with all the changes you have made so that the maintainer can track what you have done differently on each iteration. Right now you are not making any of this easy on me so I ask that you stop submitting new patches willy nilly until we have actualy discussed each item, and we decide what to do. I also ask that you keep track of each change you are making on each new patch iteration on the patch after the --- lines and before the patch, so I can easily tell all the changes you have made on each new iteration. Luis
