On Tue, Mar 29, 2022 at 2:38 PM HAGIO KAZUHITO(萩尾 一仁) <[email protected]> wrote:
> -----Original Message----- > > > Lianbo Jiang <[email protected]> writes: > > > > > > > The commit <cd8954023bd4> broke crash-utility on s390x and got the > > > > following error: > > > > > > > > crash: cannot resolve ".rodata" > > > > > > > > The reason is that all symbols containing a "." may be filtered out > > > > on s390x. To prevent the current failure, a simple way is to check > > > > whether the symbol ".rodata" exists before calculating the value of > > > > a symbol. > > > > > > > > Fixes: cd8954023bd4 ("kernel: fix start-up time degradation caused > by strings command") > > > > Reported-by: Alexander Egorenkov <[email protected]> > > > > Signed-off-by: Lianbo Jiang <[email protected]> > > > > --- > > > > kernel.c | 3 +++ > > > > 1 file changed, 3 insertions(+) > > > > > > > > diff --git a/kernel.c b/kernel.c > > > > index 92434a3ffe2d..b504564846c7 100644 > > > > --- a/kernel.c > > > > +++ b/kernel.c > > > > @@ -11790,6 +11790,9 @@ int get_linux_banner_from_vmlinux(char > *buf, size_t size) > > > > struct bfd_section *sect; > > > > long offset; > > > > > > > > + if (!symbol_exists(".rodata")) > > > > + return FALSE; > > > > + > > > > sect = bfd_get_section_by_name(st->bfd, ".rodata"); > > > > if (!sect) > > > > return FALSE; > > > > -- > > > > 2.20.1 > > > > > > thanks! This works on s390x. > > > > > > Sorry, my reply was truncated. How about the following changes? > > > > > > > > > diff --git a/kernel.c b/kernel.c > > > index 92434a3ffe2d..b504564846c7 100644 > > > --- a/kernel.c > > > +++ b/kernel.c > > > @@ -11790,6 +11790,9 @@ int get_linux_banner_from_vmlinux(char *buf, > size_t size) > > > struct bfd_section *sect; > > > long offset; > > > > > > + if (!symbol_exists(".rodata")) > > kernel_symbol_exists() is better. > Good understanding, Kazu. Looks good to me. > > > + return FALSE; > > > + > > > sect = bfd_get_section_by_name(st->bfd, ".rodata"); > > > if (!sect) > > > return FALSE; > > > diff --git a/s390.c b/s390.c > > > index 078b1a25724e..42f5cc63ae52 100644 > > > --- a/s390.c > > > +++ b/s390.c > > > @@ -442,6 +442,9 @@ s390_verify_symbol(const char *name, ulong value, > char type) > > > if (strstr(name, "L2\002") == name) > > > return FALSE; > > > > > > + if (STREQ(name, ".rodata")) > > > + return TRUE; > > > + > > > /* throw away all symbols containing a '.' */ > > > for(i = 0; i < strlen(name);i++){ > > > if(name[i] == '.') > > > diff --git a/s390x.c b/s390x.c > > > index c07d283d7f52..d7ee3755fc0b 100644 > > > --- a/s390x.c > > > +++ b/s390x.c > > > @@ -1087,6 +1087,9 @@ s390x_verify_symbol(const char *name, ulong > value, char type) > > > if (strstr(name, "L2\002") == name) > > > return FALSE; > > > > > > + if (STREQ(name, ".rodata")) > > > + return TRUE; > > > + > > > /* throw away all symbols containing a '.' */ > > > for(i = 0; i < strlen(name);i++){ > > > if(name[i] == '.') > > > > Looks good to me. > > Tested on s390x, works with get_linux_banner_from_vmlinux(). > > I also could not find the reason why it throws away those symbols in the > changelog [1] > and the list archive [2]. So I think it might be safe to not change it.. > but > as Alex is ok with this, > > Acked-by: Kazuhito Hagio <[email protected]> Thanks for the response, I will apply this later. > > [1] https://crash-utility.github.io/crash.changelog.html > [2] e.g. > https://www.google.com/search?q=site%3Ahttps%3A%2F%2Flistman.redhat.com%2F+%22s390x_verify_symbol%22 > > Thanks! > Kazu > >
-- Crash-utility mailing list [email protected] https://listman.redhat.com/mailman/listinfo/crash-utility Contribution Guidelines: https://github.com/crash-utility/crash/wiki
