search

Dr-Noob / cpufetch

Simple yet fancy CPU architecture fetching tool
GNU General Public License v2.0
1.83k stars 100 forks source link

[PPC] Warnings and Errors #178

Closed bbonev closed 1 year ago

bbonev commented 1 year ago

IBM POWER8, Debian testing, gcc version 12.2.0

processor   : 159
cpu     : POWER8 (architected), altivec supported
clock       : 3425.000000MHz
revision    : 2.1 (pvr 004b 0201)

timebase    : 512000000
platform    : pSeries
model       : IBM,8247-22L
machine     : CHRP IBM,8247-22L
MMU     : Hash
src/common/printer.c: In function ‘replace_bgbyfg_color’:
src/common/printer.c:306:29: warning: the comparison will always evaluate as ‘false’ for the address of ‘color_ascii’ will never be NULL [-Waddress]
  306 |     if(logo->color_ascii[i] == NULL) break;
src/common/printer.c: In function ‘print_ascii_generic’:
src/common/printer.c:487:23: error: ‘ATTRIBUTE_CPU_NUM’ undeclared (first use in this function); did you mean ‘ATTRIBUTE_PEAK’?
  487 |       if(attr_type == ATTRIBUTE_CPU_NUM) {
src/common/printer.c: In function ‘print_cpufetch_ppc’:
src/common/printer.c:693:3: error: too few arguments to function ‘print_ascii_generic’
  693 |   print_ascii_generic(art, longest_attribute, term->w, attribute_fields);
cc1: note: unrecognized command-line option ‘-Wno-language-extension-token’ may have been intended to silence earlier diagnostics

HTH

PS. Just checked that latest git compiles, but does not work: (maybe that is related to #153).

$ ./cpufetch 
double free or corruption (out)
Aborted

valgrind says:

==25298== Invalid read of size 4
==25298==    at 0x189040: get_topology_info (in /home/bbonev/cpufetch/cpufetch)
==25298==    by 0x18972B: get_cpu_info (in /home/bbonev/cpufetch/cpufetch)
==25298==    by 0x1819B3: main (in /home/bbonev/cpufetch/cpufetch)
==25298==  Address 0x4c9590c is 4 bytes before a block of size 640 alloc'd
==25298==    at 0x48A5418: malloc (vg_replace_malloc.c:381)
==25298==    by 0x188AD3: emalloc (in /home/bbonev/cpufetch/cpufetch)
==25298==    by 0x188FC7: get_topology_info (in /home/bbonev/cpufetch/cpufetch)
==25298==    by 0x18972B: get_cpu_info (in /home/bbonev/cpufetch/cpufetch)
==25298==    by 0x1819B3: main (in /home/bbonev/cpufetch/cpufetch)
==25298== 
==25298== Invalid write of size 4
==25298==    at 0x189048: get_topology_info (in /home/bbonev/cpufetch/cpufetch)
==25298==    by 0x18972B: get_cpu_info (in /home/bbonev/cpufetch/cpufetch)
==25298==    by 0x1819B3: main (in /home/bbonev/cpufetch/cpufetch)
==25298==  Address 0x4c9590c is 4 bytes before a block of size 640 alloc'd
==25298==    at 0x48A5418: malloc (vg_replace_malloc.c:381)
==25298==    by 0x188AD3: emalloc (in /home/bbonev/cpufetch/cpufetch)
==25298==    by 0x188FC7: get_topology_info (in /home/bbonev/cpufetch/cpufetch)
==25298==    by 0x18972B: get_cpu_info (in /home/bbonev/cpufetch/cpufetch)
==25298==    by 0x1819B3: main (in /home/bbonev/cpufetch/cpufetch)
==25298== 
==25298== Conditional jump or move depends on uninitialised value(s)
==25298==    at 0x4945D20: ____strtol_l_internal (strtol_l.c:431)
==25298==    by 0x4945B8B: strtol (strtol.c:106)
==25298==    by 0x183EE3: get_num_caches_from_files (in /home/bbonev/cpufetch/cpufetch)
==25298==    by 0x18442F: get_num_caches_by_level (in /home/bbonev/cpufetch/cpufetch)
==25298==    by 0x188DE7: get_cache_info (in /home/bbonev/cpufetch/cpufetch)
==25298==    by 0x18973B: get_cpu_info (in /home/bbonev/cpufetch/cpufetch)
==25298==    by 0x1819B3: main (in /home/bbonev/cpufetch/cpufetch)

                                                              Microarchitecture:   POWER8
                                                              Technology:          22nm
                                                              Max Frequency:       Unknown
                                                              Cores:               0 cores
                                                              Altivec:             Yes
                                                              L1i Size:            32KB (640KB Total)
                                                              L1d Size:            64KB (1.25MB Total)
                                                              L2 Size:             512KB (10MB Total)
                                                              L3 Size:             8MB (160MB Total)
                                                              Peak Performance:    Unknown

==25298== 
==25298== HEAP SUMMARY:
==25298==     in use at exit: 2,676,735 bytes in 2,060 blocks
==25298==   total heap usage: 6,243 allocs, 4,183 frees, 4,295,783 bytes allocated
==25298== 
==25298== LEAK SUMMARY:
==25298==    definitely lost: 2,648,433 bytes in 659 blocks
==25298==    indirectly lost: 28,302 bytes in 1,401 blocks
==25298==      possibly lost: 0 bytes in 0 blocks
==25298==    still reachable: 0 bytes in 0 blocks
==25298==         suppressed: 0 bytes in 0 blocks
==25298== Rerun with --leak-check=full to see details of leaked memory
==25298== 
==25298== Use --track-origins=yes to see where uninitialised values come from
==25298== For lists of detected and suppressed errors, rerun with: -s
==25298== ERROR SUMMARY: 2880 errors from 3 contexts (suppressed: 0 from 0)
Dr-Noob commented 1 year ago

Many thanks for the info.

To make this much more useful, I would need the exact commit in which you find both the compilation and runtime errors (e.g., the line if(logo->color_ascii[i] == NULL) break; does not exist in src/common/printer.c in the last commit). To find the commit of the program giving you the runtime error, you should be fine running ./cpufetch -V.

bbonev commented 1 year ago

Compile error is from 1.3 release (can be ignored). Valgrind is on git HEAD.

Dr-Noob commented 1 year ago

Ah, okay.

Do you mind running valgrind again but with cpufetch compiled with make debug to enable debug information? Valgrind says the issue is in get_topology_info, but that is a quite long function.

bbonev commented 1 year ago

Sure:

$ make clean
$ make debug
cc -Wall -Wextra -pedantic -DARCH_PPC -std=gnu99 -fstack-protector-all -Wno-language-extension-token -g -O0 -Wfloat-equal -Wshadow -Wpointer-arith -Wstrict-prototypes -DGIT_FULL_VERSION=\""v1.03-49-gbec4"\" src/common/main.c src/common/cpu.c src/common/udev.c src/common/printer.c src/common/args.c src/common/global.c src/ppc/ppc.c src/ppc/uarch.c src/ppc/udev.c -o cpufetch
$ valgrind ./cpufetch 
==145729== Memcheck, a memory error detector
==145729== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==145729== Using Valgrind-3.19.0 and LibVEX; rerun with -h for copyright info
==145729== Command: ./cpufetch
==145729== 
==145729== Invalid read of size 4
==145729==    at 0x18961C: get_topology_info (ppc.c:77)
==145729==    by 0x189D23: get_cpu_info (ppc.c:179)
==145729==    by 0x182253: main (main.c:101)
==145729==  Address 0x4c9590c is 4 bytes before a block of size 640 alloc'd
==145729==    at 0x48A5418: malloc (vg_replace_malloc.c:381)
==145729==    by 0x188E43: emalloc (global.c:132)
==145729==    by 0x18959F: get_topology_info (ppc.c:72)
==145729==    by 0x189D23: get_cpu_info (ppc.c:179)
==145729==    by 0x182253: main (main.c:101)
==145729== 
==145729== Invalid write of size 4
==145729==    at 0x189624: get_topology_info (ppc.c:77)
==145729==    by 0x189D23: get_cpu_info (ppc.c:179)
==145729==    by 0x182253: main (main.c:101)
==145729==  Address 0x4c9590c is 4 bytes before a block of size 640 alloc'd
==145729==    at 0x48A5418: malloc (vg_replace_malloc.c:381)
==145729==    by 0x188E43: emalloc (global.c:132)
==145729==    by 0x18959F: get_topology_info (ppc.c:72)
==145729==    by 0x189D23: get_cpu_info (ppc.c:179)
==145729==    by 0x182253: main (main.c:101)
==145729== 
==145729== Conditional jump or move depends on uninitialised value(s)
==145729==    at 0x4945D20: ____strtol_l_internal (strtol_l.c:431)
==145729==    by 0x4945B8B: strtol (strtol.c:106)
==145729==    by 0x18469B: get_num_caches_from_files (udev.c:217)
==145729==    by 0x184B47: get_num_caches_by_level (udev.c:273)
==145729==    by 0x18921B: get_cache_info (ppc.c:25)
==145729==    by 0x189D37: get_cpu_info (ppc.c:180)
==145729==    by 0x182253: main (main.c:101)
==145729== 

                                                              Microarchitecture:   POWER8
                                                              Technology:          22nm
                                                              Max Frequency:       Unknown
                                                              Cores:               0 cores
                                                              Altivec:             Yes
                                                              L1i Size:            32KB (640KB Total)
                                                              L1d Size:            64KB (1.25MB Total)
                                                              L2 Size:             512KB (10MB Total)
                                                              L3 Size:             8MB (160MB Total)
                                                              Peak Performance:    Unknown

==145729== 
==145729== HEAP SUMMARY:
==145729==     in use at exit: 2,676,735 bytes in 2,060 blocks
==145729==   total heap usage: 6,243 allocs, 4,183 frees, 4,295,783 bytes allocated
==145729== 
==145729== LEAK SUMMARY:
==145729==    definitely lost: 2,648,433 bytes in 659 blocks
==145729==    indirectly lost: 28,302 bytes in 1,401 blocks
==145729==      possibly lost: 0 bytes in 0 blocks
==145729==    still reachable: 0 bytes in 0 blocks
==145729==         suppressed: 0 bytes in 0 blocks
==145729== Rerun with --leak-check=full to see details of leaked memory
==145729== 
==145729== Use --track-origins=yes to see where uninitialised values come from
==145729== For lists of detected and suppressed errors, rerun with: -s
==145729== ERROR SUMMARY: 2880 errors from 3 contexts (suppressed: 0 from 0)

If that will help, I can provide you access

Dr-Noob commented 1 year ago

Okay, I was suspecting the error was in ppc.c:77 and there it is.

I have pushed a new commit in the bugfix2 branch. Can you run ./cpufetch -v? It should give an error message somewhere, probably in check_package_ids_integrity. If it does not, I would really appreciate access to your machine to figure out what's wrong.

bbonev commented 1 year ago 
$ valgrind ./cpufetch 
==147126== Memcheck, a memory error detector
==147126== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==147126== Using Valgrind-3.19.0 and LibVEX; rerun with -h for copyright info
==147126== Command: ./cpufetch
==147126== 
==147126== Invalid read of size 4
==147126==    at 0x18961C: get_topology_info (ppc.c:77)
==147126==    by 0x189D23: get_cpu_info (ppc.c:179)
==147126==    by 0x182253: main (main.c:101)
==147126==  Address 0x4c9590c is 4 bytes before a block of size 640 alloc'd
==147126==    at 0x48A5418: malloc (vg_replace_malloc.c:381)
==147126==    by 0x188E43: emalloc (global.c:132)
==147126==    by 0x18959F: get_topology_info (ppc.c:72)
==147126==    by 0x189D23: get_cpu_info (ppc.c:179)
==147126==    by 0x182253: main (main.c:101)
==147126== 
==147126== Invalid write of size 4
==147126==    at 0x189624: get_topology_info (ppc.c:77)
==147126==    by 0x189D23: get_cpu_info (ppc.c:179)
==147126==    by 0x182253: main (main.c:101)
==147126==  Address 0x4c9590c is 4 bytes before a block of size 640 alloc'd
==147126==    at 0x48A5418: malloc (vg_replace_malloc.c:381)
==147126==    by 0x188E43: emalloc (global.c:132)
==147126==    by 0x18959F: get_topology_info (ppc.c:72)
==147126==    by 0x189D23: get_cpu_info (ppc.c:179)
==147126==    by 0x182253: main (main.c:101)
==147126== 
==147126== Conditional jump or move depends on uninitialised value(s)
==147126==    at 0x4945D20: ____strtol_l_internal (strtol_l.c:431)
==147126==    by 0x4945B8B: strtol (strtol.c:106)
==147126==    by 0x18469B: get_num_caches_from_files (udev.c:217)
==147126==    by 0x184B47: get_num_caches_by_level (udev.c:273)
==147126==    by 0x18921B: get_cache_info (ppc.c:25)
==147126==    by 0x189D37: get_cpu_info (ppc.c:180)
==147126==    by 0x182253: main (main.c:101)
==147126== 

                                                              Microarchitecture:   POWER8
                                                              Technology:          22nm
                                                              Max Frequency:       Unknown
                                                              Cores:               0 cores
                                                              Altivec:             Yes
                                                              L1i Size:            32KB (640KB Total)
                                                              L1d Size:            64KB (1.25MB Total)
                                                              L2 Size:             512KB (10MB Total)
                                                              L3 Size:             8MB (160MB Total)
                                                              Peak Performance:    Unknown

==147126== 
==147126== HEAP SUMMARY:
==147126==     in use at exit: 2,676,735 bytes in 2,060 blocks
==147126==   total heap usage: 6,243 allocs, 4,183 frees, 4,295,783 bytes allocated
==147126== 
==147126== LEAK SUMMARY:
==147126==    definitely lost: 2,648,433 bytes in 659 blocks
==147126==    indirectly lost: 28,302 bytes in 1,401 blocks
==147126==      possibly lost: 0 bytes in 0 blocks
==147126==    still reachable: 0 bytes in 0 blocks
==147126==         suppressed: 0 bytes in 0 blocks
==147126== Rerun with --leak-check=full to see details of leaked memory
==147126== 
==147126== Use --track-origins=yes to see where uninitialised values come from
==147126== For lists of detected and suppressed errors, rerun with: -s
==147126== ERROR SUMMARY: 2880 errors from 3 contexts (suppressed: 0 from 0)
Dr-Noob commented 1 year ago

Everything should be working now.

The problem was caused due to virtualization. Like in x86, Linux also exports less information via /sys, which can fool cpufetch into very weird behaviors. I have fixed it to protect it against this situation. Additionally, I have added support for:

So the output is much better now. Please let me know if you find any issues with the last patches (I have merged everything into master).

bbonev commented 1 year ago

Yes, it looks better - no crash and saner results.

Still valgrind reports Conditional jump or move depends on uninitialised value(s) - not sure if that is a false positive, but it looks like not to be...

Dr-Noob commented 1 year ago

Thanks for the info, it wasn't a false positive. I have fixed it in the last commit. I'm closing the issue and marking this as completed, feel free to open another one if you have further comments.

As always, thanks for your suggestions!