Wrong srec_info output when using the last 32-bit address
Closed this issue · 4 comments
srec_info prints the last 32-bit address wrongly in address range end.
This is a regression introduced between version 1.64 and 1.65.0.
Note: srec_info can generate data itself, but that would only work for this first case, it wouldn't show the misbehavior of the second case below.
srec_cat -generate 0xFFFFFFFF 0x100000000 -constant 0 -header "" -start-addr 0 | srec_info
Output is:
Format: Motorola S-Record
Execution Start Address: 00000000
Data: FFFFFFFF - FFFFFFFFFFFFFFFF
Output should be:
Format: Motorola S-Record
Execution Start Address: 00000000
Data: FFFFFFFF - FFFFFFFF
Additionally srec_info doesn't fill other data ranges up to 8 nibbles if the last 32-bit address is used.
This is not a regression, it was already the case with version 1.64.
srec_cat -generate 0x0 0x1 -constant 0 -generate 0xFFFFFFFF 0x100000000 -constant 0 -header "" -start-addr 0 | srec_info
Output is:
Format: Motorola S-Record
Execution Start Address: 00000000
Data: 0000 - 0000
FFFFFFFF - FFFFFFFFFFFFFFFF
Output should be:
Format: Motorola S-Record
Execution Start Address: 00000000
Data: 00000000 - 00000000
FFFFFFFF - FFFFFFFF
In the newly introduced verbose mode, the additional fields also can't cope with the last address.
srec_cat -generate 0x0 0x1 -constant 0 -generate 0xFFFFFFFF 0x100000000 -constant 0 -header "" -start-addr 0 | srec_info -verbose
Output is:
Format: Motorola S-Record
Execution Start Address: 00000000
Data: 0000 - 0000 (0001)
FFFFFFFF - FFFFFFFFFFFFFFFF (FFFFFFFF00000001)
Filled: FFFFFFFF00000002
Allocated: inf% Holes: -inf%
Output should be:
Format: Motorola S-Record
Execution Start Address: 00000000
Data: 00000000 - 00000000 (00000001)
FFFFFFFF - FFFFFFFF (00000001)
Filled: 00000002
Allocated: 0.00% Holes: 100.00%
Compare with the following command using the second to last address with the correct output already.
srec_cat -generate 0x0 0x1 -constant 0 -generate 0xFFFFFFFE 0xFFFFFFFF -constant 0 -header "" -start-addr 0 | srec_info -verbose
Format: Motorola S-Record
Execution Start Address: 00000000
Data: 00000000 - 00000000 (00000001)
FFFFFFFE - FFFFFFFE (00000001)
Filled: 00000002
Allocated: 0.00% Holes: 100.00%
[EDIT - I wrote this a bit too quickly, I thought I saw an implicit cast to a signed type in there, but probably wrong.]
main.cc:154 may be problematic:
unsigned long lo = tmp.get_lowest();
snprintf(buf, sizeof(buf), "%0*lX", prec, lo);
std::cout << buf << " - ";
unsigned long hi = tmp.get_highest();
number_bytes += hi - lo;
snprintf(buf, sizeof(buf), "%0*lX", prec, hi - 1);
get_lowest()
returns a "data_t" value, typedef'd as a uint32. Cool. But that snprintf uses an unsigned long
which can happen to be a uint64.
It's hard to use generic formatters like %lX when using typdef'ed datatypes... The anal way would be to have explicit uint32 / uint64 everywhere, and use the awful "%PRIU32" formatters (those are C99 <inttypes.h> , I hope modern c++ has the equivalent ?)
It took me a while to understand the problem, because neither of the examples worked with the old srec_cat versions. So this is what the endless loop looked like, before being fixed in #6.
So the essence for me is: That patch was initially developed on 1.64 where it worked correctly and now after rebasing to 1.65 the output is wrong.
I did backport the patch and tried out the first example command line.
The first commit it fails is 16cf9cd (Fix error length represenation in verbose srec_info output. Dropped range as it added no value and was confusing., 2022-10-13).
The code snippet in @fenugrec's post is the right one. The hi
address gets 0.
And the unsigned long
on 64-bit Linux (not on 32-bit Linux and not on Win32/64) is a 64-bit data type. so the result of the substraction is an integer underflow which result to FFFFFFFFFFFFFFFF
instead of FFFFFFFF
.
Replacing all unsigned long
by uint32_t
solves the problem for me. But I would also cast any arithmetic operation (like such substractions) to avoid any kind of integer promotion.
It took me a while to understand the problem, because neither of the examples worked with the old srec_cat versions.
I used srec_cat to generate the input data because of these reasons:
- more readable than some input file format
- easier handling without having to create an input file
For testing I simply used srec_cat from current master (bugs fixed) and only used srec_info from the commit to be tested.