Opened 13 years ago
Closed 10 years ago
#536 closed defect (fixed)
IZ2635: Fails to build libs/uti/sge_getloadavg.c with gcc 4.3.1
Reported by: | paulmillar | Owned by: | |
---|---|---|---|
Priority: | normal | Milestone: | |
Component: | sge | Version: | current |
Severity: | minor | Keywords: | build |
Cc: |
Description
[Imported from gridengine issuezilla http://gridengine.sunsource.net/issues/show_bug.cgi?id=2635]
Issue #: 2635 Platform: All Reporter: paulmillar (paulmillar) Component: gridengine OS: All Subcomponent: build Version: current CC: None defined Status: NEW Priority: P3 Resolution: Issue type: DEFECT Target milestone: --- Assigned to: andreas (andreas) QA Contact: andreas URL: * Summary: Fails to build libs/uti/sge_getloadavg.c with gcc 4.3.1 Status whiteboard: Attachments: Date/filename: Description: Submitted by: Wed Jul 2 01:53:00 -0700 2008: 2635.diff A source diff that should fix the problem (text/plain) andreas Wed Jul 2 01:59:00 -0700 2008: 2635.diff New try (former diff was buggy) (text/plain) andreas Issue 2635 blocks: Votes for issue 2635: Opened: Tue Jul 1 16:36:00 -0700 2008 ------------------------ I tried to build gridengine from cvs on Debian sid. The current HEAD failed whilst building the libraries, in particular with libs/uti/sge_getloadavg.c. The problem was with line 1306 and I've copied the output below: _________C_O_R_E__S_Y_S_T_E_M_____________ gcc -O3 -Wall -Werror -Wstrict-prototypes -D__GRIDENGINE_FD_SETSIZE=8192 -DLINUX -DLINUX86 -DLINUX86_26 -D_GNU_SOURCE -DGETHOSTBYNAME_R6 -DGETHOSTBYADDR_R8 -DLOAD_OPENSSL -I/vol2/SW/db-4.4.20/lx26-x86/include/ -DSGE_ARCH_STRING=lx26-x86 -DTARGET_32BIT -DSPOOLING_dynamic -DSECURE -I/vol2/tools/SW/openssl-0.9.8g-origin/lx26-x86/include -Wno-strict-aliasing -D_FILE_OFFSET_BITS=64 -DCOMPILE_DC -D__SGE_COMPILE_WITH_GETTEXT__ -D__SGE_NO_USERMAPPING__ -I../common -I../libs -I../libs/uti -I../libs/juti -I../libs/gdi -I../libs/japi -I../libs/sgeobj -I../libs/cull -I../libs/rmon -I../libs/comm -I../libs/comm/lists -I../libs/sched -I../libs/evc -I../libs/evm -I../libs/mir -I../libs/lck -I../daemons/common -I../daemons/qmaster -I../daemons/execd -I../daemons/schedd -I../clients/common -I. -I/usr/lib/jvm/java-6-sun/include -I/usr/lib/jvm/java-6-sun/include/linux -fPIC -c ../libs/uti/sge_getloadavg.c cc1: warnings being treated as errors ../libs/uti/sge_getloadavg.c: In function 'get_cpu_load': ../libs/uti/sge_getloadavg.c:1306: error: array subscript is above array bounds ../libs/uti/sge_getloadavg.c:1306: error: array subscript is above array bounds make: *** [sge_getloadavg.o] Error 1 I've not traced the logic of the function, but the code doesn't pass the "sniff test". I've copied a patch that fixes this issue, allowing the compilation to progress, although it failed later on. Index: libs/uti/sge_getloadavg.c =================================================================== RCS file: /cvs/gridengine/source/libs/uti/sge_getloadavg.c,v retrieving revision 1.38 diff -u -r1.38 sge_getloadavg.c --- libs/uti/sge_getloadavg.c 15 Apr 2008 12:40:54 -0000 1.38 +++ libs/uti/sge_getloadavg.c 1 Jul 2008 23:19:22 -0000 @@ -1302,10 +1302,11 @@ /* calculate percentages based on overall change, rounding up */ half_total = total_change / 2l; for (i = 0; i < cnt; i++) { - *out = ((double)((*diffs++ * 1000 + half_total) / total_change))/10; + *out = ((double)((*diffs * 1000 + half_total) / total_change))/10; DPRINTF(("diffs: %lu half_total: %lu total_change: %lu -> %f", *diffs, half_total, total_change, *out)); out++; + diffs++; } DEXIT; Naturally, someone who understands the precise semantics of this function should review the patch. Cheers, Paul. PS. Can one attach patches to to bug with this issue-tracker? I'd guess that posting patches in-line is fragile. ------- Additional comments from andreas Wed Jul 2 01:53:01 -0700 2008 ------- Created an attachment (id=176) A source diff that should fix the problem ------- Additional comments from andreas Wed Jul 2 01:59:21 -0700 2008 ------- Created an attachment (id=177) New try (former diff was buggy) ------- Additional comments from andreas Wed Jul 2 02:02:08 -0700 2008 ------- Paul, could you try the second diff that I attached to this issue and let me know the result? Note, the first one was buggy, since DPRINTF expressions are evaluated in monitoring mode only. For that reason increments must be done outside the DPRINTF statements. Regards, Andreas ------- Additional comments from paulmillar Wed Jul 2 14:48:01 -0700 2008 ------- Hi Andreas, Thanks for looking into this. Both patches look broken to me. The first patch *only* increments the two ptrs inside the DPRINTF, which (as you say) is broken if monitoring is switched off; the second patch increments both inside and outside the DPRINT, which is broken if monitoring is switched on! Could you have another look at my patch? I still believe this is the correct fix. Cheers, Paul. PS. Is it possible to use unified output for diffs ("cvs diff -u")? I find these easier to read. ------- Additional comments from paulmillar Mon Jul 14 17:06:04 -0700 2008 ------- Hi Andreas, A couple of updates on this issue: The first point is I've tried the second version of the patch, as you recommended. At first it seemed to work; however, I was concerned that the compiler was somehow factoring out the DPRINTF macro (hence the diffs++ and out++ within the DPRINTF macro are never evaluated). This would hide the problem until someone attempts to compile with an enabled DPRINTF. To test this, I replaced the DPRINTF macro with a simple printf and the compilation broke again: gcc -O3 -Wall -Werror -Wstrict-prototypes -D__GRIDENGINE_FD_SETSIZE=8192 [... many more arguments ...] -I/usr/lib/jvm/java-6-sun-1.6.0.07/jre/include/linux -fPIC -c ../libs/uti/sge_getloadavg.c cc1: warnings being treated as errors ../libs/uti/sge_getloadavg.c: In function 'get_cpu_load': ../libs/uti/sge_getloadavg.c:1305: error: array subscript is above array bounds ../libs/uti/sge_getloadavg.c:1305: error: array subscript is above array bounds ../libs/uti/sge_getloadavg.c:1305: error: array subscript is above array bounds ../libs/uti/sge_getloadavg.c:1305: error: array subscript is above array bounds make: *** [sge_getloadavg.o] Error 1 I believe this demonstrates that the second version (of the patch) is still broken --- although I know why DPRINTF is not having any affect: is DPRINTF not available for the uti library? The second point is that I've just noticed that there's a function called percentages_new() in the same file that is similar to percentages(). The patches so far only fix percentages() and not percentages_new(). The latter looks to have the same problem as the former, but was not picked up by gcc as the code is wrapped by some preprocessor tests for (I believe) compilation architecture. HTH, Paul.
Attachments (2)
Change History (3)
Changed 10 years ago by admin
Changed 10 years ago by admin
comment:1 Changed 10 years ago by dlove
- Resolution set to fixed
- Severity set to minor
- Status changed from new to closed
Note: See
TracTickets for help on using
tickets.
Seems to be fixed by [2819].