On Sat, Oct 2, 2021 at 12:10 AM Philipp Rudo <prudo@xxxxxxxxxx> wrote:
>
> Hi Tao,
> Hi Lianbo,
>
> sorry for the late response.
>
> On Wed, 29 Sep 2021 15:46:19 +0800
> lijiang <lijiang@xxxxxxxxxx> wrote:
>
> > > > > Currently, the macro is used twice in the symbols.c. This change seems
> > > > > not complicated. Any thoughts?
> > > >
> > > > do I understand your suggestion correct, you propose to replace the
> > > >
> > > > #define SYMNAME_HASH_INDEX(name) ...
> > > >
> > > > in defs.h by something like
> > > >
> > > > static unsigned long long SYMNAME_HASH_INDEX(const unsigned char * const name) {
> > > > return (name[0] ^ (name[strlen(name)-1] * name[strlen(name)/2]) % SYMNAME_HASH);
> > > > }
> > > >
> > > > in symbols.c? If so, I think that should be fine.
> > > >
> > Yes, you are right, Philipp.
> >
> > >
> > > Please correct me if I'm wrong. I don't think the function can work.
> > > Let's say name[0] ^ (name[strlen(name)-1] * name[strlen(name)/2]) ==
> > > -1, then we will have:
> > >
> > > static unsigned long long SYMNAME_HASH_INDEX(const unsigned char * const name) {
> > > return (-1) % 512;
> > > }
> > >
> > > The returned value is a very large number, and will overflow the array.
>
> @Tao: I don't think that should be possible here. In the function I
> have defined name as _unsigned_ char*. So the calculation should never
> become negative. But to be honest I haven't tested it.
>
> > No, this is a modulo operation, and its result will never exceed '512' anyway.
> > (unsigned long long)(-1) % 512 = 511
>
> @Lianbo: I don't think that's true. When the modulo is used with an
> negative dividend the result becomes negative. When this then gets
> casted to an unsigend integer type the result will become very large.
> That's also what a quick test showed me
>
> printf("%lli\n", ((-1LL) % 512)); --> -1
> printf("%llu\n", (unsigned long long) ((-1LL) % 512)); --> 18446744073709551615
>
Thank you for the reply, Tao and Philipp. Let's go back to the problem
itself and continue our discussion.
Does the following change work for you?
---
defs.h | 2 --
symbols.c | 18 +++++++++++++++---
2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/defs.h b/defs.h
index cbd45e52f9da..d7b9bcc89878 100644
--- a/defs.h
+++ b/defs.h
@@ -2728,8 +2728,6 @@ struct downsized {
(((vaddr) >> machdep->pageshift) % SYMVAL_HASH)
#define SYMNAME_HASH (512)
-#define SYMNAME_HASH_INDEX(name) \
- ((name[0] ^ (name[strlen(name)-1] * name[strlen(name)/2])) % SYMNAME_HASH)
#define PATCH_KERNEL_SYMBOLS_START ((char *)(1))
#define PATCH_KERNEL_SYMBOLS_STOP ((char *)(2))
diff --git a/symbols.c b/symbols.c
index 69dccdb09d5f..8a2230df7712 100644
--- a/symbols.c
+++ b/symbols.c
@@ -1127,6 +1127,18 @@ symname_hash_init(void)
st->__per_cpu_end = sp->value;
}
+unsigned int symname_hash_index(unsigned char *name)
+{
+ unsigned int len, value;
+
+ len = strlen(name);
+ if (!len)
+ error(FATAL, "The length of the symbol name is zero!\n");
+
+ value = name[len-1] * name[len/2];
+
+ return (name[0] ^ value) % (unsigned int)SYMNAME_HASH;
+}
/*
* Install a single static kernel symbol into the symname_hash.
*/
@@ -1134,9 +1146,9 @@ static void
symname_hash_install(struct syment *spn)
{
struct syment *sp;
- int index;
+ unsigned int index;
- index = SYMNAME_HASH_INDEX(spn->name);
+ index = symname_hash_index(spn->name);
spn->cnt = 1;
if ((sp = st->symname_hash[index]) == NULL)
@@ -1165,7 +1177,7 @@ symname_hash_search(char *name)
{
struct syment *sp;
- sp = st->symname_hash[SYMNAME_HASH_INDEX(name)];
+ sp = st->symname_hash[symname_hash_index(name)];
while (sp) {
if (STREQ(sp->name, name))
--
2.17.1
Thanks.
Lianbo
> > --
> > Crash-utility mailing list
> > Crash-utility@xxxxxxxxxx
> > https://listman.redhat.com/mailman/listinfo/crash-utility
> >
>
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility
