Date: Sun, 14 Oct 2001 23:37:00 -0700 From: Peter Wemm <peter@wemm.org> To: Mike Smith <msmith@FreeBSD.ORG> Cc: Marcel Moolenaar <marcel@xcllnt.net>, ia64@FreeBSD.ORG, Andrew.Grover@intel.com Subject: Re: Faulty ACPI debug code [was: Re: Booting a dual ...] Message-ID: <20011015063700.EF98C3810@overcee.netplex.com.au> In-Reply-To: <200110150547.f9F5lLx04912@mass.dis.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Mike Smith wrote:
>
> I've copied the Intel ACPI CA folks on this, since I'm sure they'll be
> interested.
While on the subject, If you add printf format checking, there are some
*nasty* problems in the debug printfs. They fixed about 2/3 of them a few
months ago but left the others alone. They're still trying to printf
a 64 bit int with %p on the i386 platform (which causes stack misalignment).
ie:
--- acutils.h 2001/10/04 23:12:11 1.1.1.11
+++ acutils.h 2001/10/15 06:00:36
@@ -436,5 +436,5 @@
ACPI_DEBUG_PRINT_INFO *DbgInfo,
char *Format,
- ...);
+ ...) __attribute__ ((format (printf, 4, 5)));
void
@@ -444,5 +444,5 @@
ACPI_DEBUG_PRINT_INFO *DbgInfo,
char *Format,
- ...);
+ ...) __attribute__ ((format (printf, 4, 5)));
Additional warnings about the function in question:
dsopcode.c: In function `AcpiDsGetRegionArguments':
dsopcode.c:325: warning: char format, different type arg (arg 5)
Line 325:
ACPI_DEBUG_PRINT ((ACPI_DB_EXEC, "[%4.4s] OpRegion Init at AML %p[%x]\n",
&Node->Name, ExtraDesc->Extra.AmlStart,
*(UINT32*) ExtraDesc->Extra.AmlStart));
typedef struct acpi_node
{
UINT32 Name; /* ACPI Name, always 4 chars per ACPI spec */
} ACPI_NAMESPACE_NODE;
This is passing an &UINT32 instead of 'char *'. This had better be
little endian.
A sample of more problems:
evevent.c:559: warning: format argument is not a pointer (arg 6)
evevent.c:559: warning: format argument is not a pointer (arg 8)
ACPI_DEBUG_PRINT ((ACPI_DB_INFO, "GPE registers: %X@%p (Blk0) %X@%p (Blk1)\n",
Gpe0RegisterCount, AcpiGbl_FADT->XGpe0Blk.Address, Gpe1RegisterCount,
AcpiGbl_FADT->XGpe1Blk.Address));
XGpe0Blk.Address and XGpe1Blk.Address are 64 bit integers, not pointers.
This causes stack misalignment.
exdump.c:348: warning: long int format, different type arg (arg 5)
exdump.c:366: warning: long int format, different type arg (arg 5)
ACPI_DEBUG_PRINT_RAW ((ACPI_DB_INFO, " value is [%ld]",
EntryDesc->Integer.Value));
The above two are moderately harmless unless you are compiling on
gcc where long == 64 bit.
exdump.c:491: warning: format argument is not a pointer (arg 5)
ACPI_DEBUG_PRINT_RAW ((ACPI_DB_INFO, " base %p Length %X\n",
EntryDesc->Region.Address, EntryDesc->Region.Length));
This is a fatal UINT64 / 32 bit %p mixup.
exprep.c:434: warning: char format, different type arg (arg 6)
exprep.c:542: warning: char format, different type arg (arg 6)
exprep.c:633: warning: char format, different type arg (arg 6)
ACPI_DEBUG_PRINT ((ACPI_DB_INFO, "set NamedObj %p (%4.4s) val = %p\n",
Node, &(Node->Name), ObjDesc));
More &UINT32 vs char *
exresolv.c:239: warning: long unsigned int format, different type arg (arg 11)
relatively harmless long / int mixup.
exresop.c:248: warning: unsigned int format, pointer arg (arg 5)
ACPI_DEBUG_PRINT ((ACPI_DB_ERROR, "Internal - null stack entry at %X\n",
StackPtr));
This should actually use %p
hwregs.c:573: warning: format argument is not a pointer (arg 6)
ACPI_DEBUG_PRINT ((ACPI_DB_IO, "PM2 control: Read %X from %p\n",
RegisterValue, ACPI_GET_ADDRESS (AcpiGbl_FADT->XPm2CntBlk.Address)));
Another fatal (for debugging) UINT64 / %p mixup
hwregs.c:583: warning: format argument is not a pointer (arg 6)
ACPI_DEBUG_PRINT ((ACPI_DB_IO, "About to write %04X to %p\n", RegisterValue,
AcpiGbl_FADT->XPm2CntBlk.Address));
same again
hwregs.c:597: warning: format argument is not a pointer (arg 6)
ACPI_DEBUG_PRINT ((ACPI_DB_IO, "PM_TIMER: Read %X from %p\n",
RegisterValue, ACPI_GET_ADDRESS (AcpiGbl_FADT->XPmTmrBlk.Address)));
and again...
nsdump.c:570: warning: unsigned int format, different type arg (arg 6)
ACPI_DEBUG_PRINT_RAW ((ACPI_DB_TABLES, " HID: %.8X, ADR: %.8X, Status: %x\n",
Info.HardwareId, Info.Address, Info.CurrentStatus));
This is an interesting one... Info.HardwareId is NATIVE_CHAR HardwareId[9];
It should probably be %p.
This is a sample of the problems that using printf type checking
turns up. I have only included a small fraction of the instances of
each problem as an example only. Some only mess up debugging output,
and cause lots of wasted time due to wild goose chases when bogus
values are reported. (printf %p of a uint64 always prints 0 on i386
with gcc (eg: on freebsd and linux))
Cheers,
-Peter
--
Peter Wemm - peter@FreeBSD.org; peter@yahoo-inc.com; peter@netplex.com.au
"All of this is for nothing if we don't go to the stars" - JMS/B5
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-ia64" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20011015063700.EF98C3810>