svn commit: r310045 - head/sys/ddb

[Bruce Evans]

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