On Wed, 14 Dec 2016, John Baldwin wrote:

On Wednesday, December 14, 2016 07:24:46 PM Bruce Evans wrote:
...
I don't see how initializing to 'val' can help.  It still gets corrupted
to unsigned, and I would expect it to make the problem much worse because
the truncation gives an even smaller value.  E.g., if val is any multiple
of 2**32, truncating it gives 0.  Without the truncation, 'val' is a good
upper bound for the offset.

I should have mentioned this change in the commit.  Other symbol resolution
APIs in the kernel follow the convention that the raw address is returned
as the offset ('diff') if no matching symbol is found.  This change made
db_search_symbol() consistent with that.

Perhaps the problem is more with the initialization of newdiff.  It is
initialized to the corrupted value in diff.  I don't know the details of
the API, but if X_db_search_symbol() uses this as an input parameter to
limit the search, it might be happier with the smaller corrupted value
than the larger one.  X_db_search_symbol() seems to have working types
and values, though still too much hard-coding of types.

Your casts to uintmax_t have no effect.  The types are already unsigned,
and the corrupted values cannot be fixed by later casts.

So I got myself into a bit of a mess and the hints are in my commit message
(but obscurely).  I am working within a branch and one of the things I had
done in this branch was to fix numerous warnings compiling DDB with a MIPS
n32 kernel.  n32 is quite special in that pointers are 32-bits while registers
are 64-bits, so db_addr_t is a uint32_t, and db_expr_t was a int64_t.  I had
"fixed" newdiff and diff to both be db_expr_t to match the type that
X_db_search_symbol() returns and that is how I got into trouble as that
changed 'diff' to be signed instead of unsigned.  I will revert the commit

That is a good fix if you commit it and keep the casts to an unsigned type.

from HEAD (though perhaps re-commit the 'val' part of initializing diff if
making that API consistent is a good thing to do).

I guess you fixed the intptr_t vs db_expr_t mismatches.

Did you look at fixing the printf formats?  I think ddb casts args to longs
too perfectly to match buggy long formats, so no errors should have been
detected at compile time.  I don't like casting to intptr_t on all arches
to support 1 strange one where this is needed, but as usual casting to
the widest type is less ugly than using PRI*.  sys/ddb has only 26 lines
of casts to long, 9 to unsigned long and 2 to long long, so fixing them
all won't be too painful.

Bruce
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to