[Bug 156840] (gcc4 O1+) perl-DBD-pg Placeholders no longer functioning
by Red Hat Bugzilla
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.
Summary: (gcc4 O1+) perl-DBD-pg Placeholders no longer functioning
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=156840
------- Additional Comments From jakub(a)redhat.com 2005-05-24 14:49 EST -------
That's complete overkill. If you look at the (very ugly) loop counting the
execsize, there is room in statement buffer for that sprintf, so the
patch I apparently forgot to provide is enough if all you care about is to make
it work.
--- dbdimp.c.jj 2005-04-06 16:40:20.000000000 -0400
+++ dbdimp.c 2005-05-24 07:40:21.000000000 -0400
@@ -1243,7 +1243,7 @@ int dbd_st_prepare_statement (sth, imp_s
for (currseg=imp_sth->seg; NULL != currseg; currseg=currseg->nextseg) {
strcat(statement, currseg->segment);
if (currseg->placeholder) {
- sprintf(statement, "%s$%d", statement,
currseg->placeholder);
+ sprintf(strchr(statement, '\0'), "$%d",
currseg->placeholder);
}
}
Calling pow in a loop is insane though:
/* The parameter itself: dollar sign plus digit(s) */
for (x=1; x<7; x++) {
if (currseg->placeholder < pow(10,x))
break;
}
if (x>=7)
croak("Too many placeholders!");
execsize += x+1;
Guess e.g. #include <limits.h> and
#define DBD_STRINGIFY_1(x) #x
#define DBD_STRINGIFY(x) DBD_STRINGIFY_1 (x)
execsize += strlen (DBD_STRINGIFY (INT_MAX)) + 2;
would be far cheaper (well, assuming CHAR_BIT 8 one can write
execsize += sizeof (int) * 3 + 2; as well). The strlen will be
optimized out by the compiler (unlike the expensive pow calls).
--
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
18 years, 11 months
[Bug 156840] (gcc4 O1+) perl-DBD-pg Placeholders no longer functioning
by Red Hat Bugzilla
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.
Summary: (gcc4 O1+) perl-DBD-pg Placeholders no longer functioning
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=156840
jakub(a)redhat.com changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEEDINFO |ASSIGNED
------- Additional Comments From jakub(a)redhat.com 2005-05-24 07:42 EST -------
Ok, finally see the bug. I was mislead by looking at second sprintf in the
routine, but that one is optimized out and there is a third one, which is
bogus:
if (currseg->placeholder) {
sprintf(statement, "%s$%d", statement,
currseg->placeholder);
}
You simply can't do this in C, see ISO C99, 7.19.6.6:
If copying takes place between objects that overlap, the behavior is undefined.
I certainly don't have time to rewrite this whole junk, so here is just
a quick fix for this exact case, though someone please rewrite this, ideally
from scratch.
--
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
18 years, 11 months
[Bug 156840] (gcc4 O1+) perl-DBD-pg Placeholders no longer functioning
by Red Hat Bugzilla
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.
Summary: (gcc4 O1+) perl-DBD-pg Placeholders no longer functioning
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=156840
------- Additional Comments From jakub(a)redhat.com 2005-05-24 06:05 EST -------
It seems to be the dbd_st_prepare_statement routine in dbdimp.c that matters.
If this routine is built with -D_FORTIFY_SOURCE=2, make test fails, with
-D_FORTIFY_SOURCE=1 succeeds. It is irrelevant whether the other .o files
are built with fortify 1 or 2, and similarly for other routines in dbdimp.c.
But looking at the assembly difference, it is really minimal:
--- /tmp/1 2005-05-24 11:29:49.000000000 +0200
+++ /tmp/2 2005-05-24 11:39:23.000000000 +0200
@@ -50,11 +50,13 @@ dbd_st_prepare_statement:
# basic block 1
movl -32(%ebp), %edi # imp_dbh,
movl 112(%edi), %eax # <variable>.prepare_number,
<variable>.prepare_number
- movl %eax, 8(%esp) # <variable>.prepare_number,
+ movl %eax, 16(%esp) # <variable>.prepare_number,
leal .LC149@GOTOFF(%ebx), %eax #, tmp109
- movl %eax, 4(%esp) # tmp109,
+ movl %eax, 12(%esp) # tmp109,
+ movl $-1, 8(%esp) #,
+ movl $1, 4(%esp) #,
movl %edx, (%esp) # D.18444,
- call sprintf@PLT #
+ call __sprintf_chk@PLT #
movl 12(%ebp), %eax # imp_sth,
movl 128(%eax), %edx # <variable>.prepare_name, temp.762
cld
@@ -282,12 +284,14 @@ dbd_st_prepare_statement:
ret
.L1348:
# basic block 27
- movl %eax, 12(%esp) # D.18478,
- movl %edi, 8(%esp) # statement,
+ movl %eax, 20(%esp) # D.18478,
+ movl %edi, 16(%esp) # statement,
leal .LC152@GOTOFF(%ebx), %eax #, tmp140
- movl %eax, 4(%esp) # tmp140,
+ movl %eax, 12(%esp) # tmp140,
+ movl $-1, 8(%esp) #,
+ movl $1, 4(%esp) #,
movl %edi, (%esp) # statement,
- call sprintf@PLT #
+ call __sprintf_chk@PLT #
.L1303:
# basic block 28
movl 12(%esi), %esi # <variable>.nextseg, currseg.772
and I don't see how that would change things (__sprintf_chk (buf, 1, -1, ...)
works like sprintf, except %n from writable memory is refused (but there is
no %n in this case and format strings are in read-only memory) and -1 length
means no length limit). 20(%esp) is still in the area used for outgoing
arguments, %ebp - %esp is 72 bytes and the lowest variable is at -48(%ebp).
The code quality of that routine is horrible, look e.g. at this junk line:
imp_sth->prepare_name[strlen(imp_sth->prepare_name)]='\0';
--
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
18 years, 11 months