Skip to content

Commit 392e6e9

Browse files
tglsfdctglsfdc
committed
Fix assorted bugs in ecpg's macro mechanism.
The code associated with EXEC SQL DEFINE was unreadable and full of bugs, notably: * It'd attempt to free a non-malloced string if the ecpg program tries to redefine a macro that was defined on the command line. * Possible memory stomp if user writes "-D=foo". * Undef'ing or redefining a macro defined on the command line would change the state visible to the next file, when multiple files are specified on the command line. (While possibly that could have been an intentional choice, the code clearly intends to revert to the original macro state; it's just failing to consider this interaction.) * Missing "break" in defining a new macro meant that redefinition of an existing name would cause an extra entry to be added to the definition list. While not immediately harmful, a subsequent undef would result in the prior entry becoming visible again. * The interactions with input buffering are subtle and were entirely undocumented. It's not that surprising that we hadn't noticed these bugs, because there was no test coverage at all of either the -D command line switch or multiple input files. This patch adds such coverage (in a rather hacky way I guess). In addition to the code bugs, the user documentation was confused about whether the -D switch defines a C macro or an ecpg one, and it failed to mention that you can write "-Dsymbol=value". These problems are old, so back-patch to all supported branches. Discussion: https://postgr.es/m/998011.1713217712@sss.pgh.pa.us
1 parent 91800af commit 392e6e9

File tree

13 files changed

+291
-81
lines changed

13 files changed

+291
-81
lines changed

‎doc/src/sgml/ecpg.sgml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5790,6 +5790,14 @@ EXEC SQL UPDATE Tbl SET col = MYNUMBER;
57905790
embedded SQL query because in this case the embedded SQL precompiler is not
57915791
able to see this declaration.
57925792
</para>
5793+
5794+
<para>
5795+
If multiple input files are named on the <command>ecpg</command>
5796+
preprocessor's command line, the effects of <literal>EXEC SQL
5797+
DEFINE</literal> and <literal>EXEC SQL UNDEF</literal> do not carry
5798+
across files: each file starts with only the symbols defined
5799+
by <option>-D</option> switches on the command line.
5800+
</para>
57935801
</sect2>
57945802

57955803
<sect2 id="ecpg-ifdef">

‎doc/src/sgml/ref/ecpg-ref.sgml

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,12 @@ PostgreSQL documentation
9393
</varlistentry>
9494

9595
<varlistentry>
96-
<term><option>-D <replaceable>symbol</replaceable></option></term>
96+
<term><option>-D <replaceable>symbol</replaceable>[=<replaceable>value</replaceable>]</option></term>
9797
<listitem>
9898
<para>
99-
Define a C preprocessor symbol.
99+
Define a preprocessor symbol, equivalently to the <command>EXEC SQL
100+
DEFINE</command> directive. If no <replaceable>value</replaceable> is
101+
specified, the symbol is defined with the value <literal>1</literal>.
100102
</para>
101103
</listitem>
102104
</varlistentry>

‎src/interfaces/ecpg/preproc/ecpg.c

Lines changed: 46 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -82,35 +82,46 @@ add_include_path(char *path)
8282
}
8383
}
8484

85+
/*
86+
* Process a command line -D switch
87+
*/
8588
static void
8689
add_preprocessor_define(char *define)
8790
{
88-
struct _defines *pd = defines;
89-
char *ptr,
90-
*define_copy = mm_strdup(define);
91+
/* copy the argument to avoid relying on argv storage */
92+
char *define_copy = mm_strdup(define);
93+
char *ptr;
94+
struct _defines *newdef;
9195

92-
defines = mm_alloc(sizeof(struct _defines));
96+
newdef = mm_alloc(sizeof(struct _defines));
9397

9498
/* look for = sign */
9599
ptr = strchr(define_copy, '=');
96100
if (ptr != NULL)
97101
{
102+
/* symbol has a value */
98103
char *tmp;
99104

100-
/* symbol has a value */
101-
for (tmp = ptr - 1; *tmp == ' '; tmp--);
105+
/* strip any spaces between name and '=' */
106+
for (tmp = ptr - 1; tmp >= define_copy && *tmp == ' '; tmp--);
102107
tmp[1] = '\0';
103-
defines->olddef = define_copy;
104-
defines->newdef = ptr + 1;
108+
109+
/*
110+
* Note we don't bother to separately malloc cmdvalue; it will never
111+
* be freed so that's not necessary.
112+
*/
113+
newdef->cmdvalue = ptr + 1;
105114
}
106115
else
107116
{
108-
defines->olddef = define_copy;
109-
defines->newdef = mm_strdup("1");
117+
/* define it as "1"; again no need to malloc it */
118+
newdef->cmdvalue = "1";
110119
}
111-
defines->pertinent = true;
112-
defines->used = NULL;
113-
defines->next = pd;
120+
newdef->name = define_copy;
121+
newdef->value = mm_strdup(newdef->cmdvalue);
122+
newdef->used = NULL;
123+
newdef->next = defines;
124+
defines = newdef;
114125
}
115126

116127
#define ECPG_GETOPT_LONG_REGRESSION 1
@@ -348,6 +359,8 @@ main(int argc, char *const argv[])
348359
{
349360
struct cursor *ptr;
350361
struct _defines *defptr;
362+
struct _defines *prevdefptr;
363+
struct _defines *nextdefptr;
351364
struct typedefs *typeptr;
352365
struct declared_list *list;
353366

@@ -385,28 +398,28 @@ main(int argc, char *const argv[])
385398
free(this);
386399
}
387400

388-
/* remove non-pertinent old defines as well */
389-
while (defines && !defines->pertinent)
401+
/* restore defines to their command-line state */
402+
prevdefptr = NULL;
403+
for (defptr = defines; defptr != NULL; defptr = nextdefptr)
390404
{
391-
defptr = defines;
392-
defines = defines->next;
393-
394-
free(defptr->newdef);
395-
free(defptr->olddef);
396-
free(defptr);
397-
}
398-
399-
for (defptr = defines; defptr != NULL; defptr = defptr->next)
400-
{
401-
struct _defines *this = defptr->next;
402-
403-
if (this && !this->pertinent)
405+
nextdefptr = defptr->next;
406+
if (defptr->cmdvalue != NULL)
404407
{
405-
defptr->next = this->next;
406-
407-
free(this->newdef);
408-
free(this->olddef);
409-
free(this);
408+
/* keep it, resetting the value */
409+
free(defptr->value);
410+
defptr->value = mm_strdup(defptr->cmdvalue);
411+
prevdefptr = defptr;
412+
}
413+
else
414+
{
415+
/* remove it */
416+
if (prevdefptr != NULL)
417+
prevdefptr->next = nextdefptr;
418+
else
419+
defines = nextdefptr;
420+
free(defptr->name);
421+
free(defptr->value);
422+
free(defptr);
410423
}
411424
}
412425

‎src/interfaces/ecpg/preproc/pgc.l

Lines changed: 89 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -69,16 +69,21 @@ char *token_start;
6969
static int state_before_str_start;
7070
static int state_before_str_stop;
7171

72-
struct _yy_buffer
72+
/*
73+
* State for handling include files and macro expansion. We use a new
74+
* flex input buffer for each level of include or macro, and create a
75+
* struct _yy_buffer to remember the previous level. There is not a struct
76+
* for the currently active input source; that state is kept in the global
77+
* variables YY_CURRENT_BUFFER, yylineno, and input_filename.
78+
*/
79+
static struct _yy_buffer
7380
{
7481
YY_BUFFER_STATE buffer;
7582
long lineno;
7683
char *filename;
7784
struct _yy_buffer *next;
7885
} *yy_buffer = NULL;
7986

80-
static char *old;
81-
8287
/*
8388
* Vars for handling ifdef/elif/endif constructs. preproc_tos is the current
8489
* nesting depth of such constructs, and stacked_if_value[preproc_tos] is the
@@ -444,6 +449,8 @@ cppline {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
444449

445450
%{
446451
/* code to execute during start of each call of yylex() */
452+
char *newdefsymbol = NULL;
453+
447454
token_start = NULL;
448455
%}
449456

@@ -1010,6 +1017,7 @@ cppline {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
10101017
}
10111018

10121019
{identifier} {
1020+
/* First check to see if it's a define symbol to expand */
10131021
if (!isdefine())
10141022
{
10151023
int kwvalue;
@@ -1198,17 +1206,23 @@ cppline {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
11981206
yytext[i+1] = '\0';
11991207

12001208

1201-
for (ptr = defines; ptr != NULL; ptr2 = ptr, ptr = ptr->next)
1209+
/* Find and unset any matching define; should be only 1 */
1210+
for (ptr = defines; ptr; ptr2 = ptr, ptr = ptr->next)
12021211
{
1203-
if (strcmp(yytext, ptr->olddef) == 0)
1212+
if (strcmp(yytext, ptr->name) == 0)
12041213
{
1205-
if (ptr2 == NULL)
1206-
defines = ptr->next;
1207-
else
1208-
ptr2->next = ptr->next;
1209-
free(ptr->newdef);
1210-
free(ptr->olddef);
1211-
free(ptr);
1214+
free(ptr->value);
1215+
ptr->value = NULL;
1216+
/* We cannot forget it if there's a cmdvalue */
1217+
if (ptr->cmdvalue == NULL)
1218+
{
1219+
if (ptr2 == NULL)
1220+
defines = ptr->next;
1221+
else
1222+
ptr2->next = ptr->next;
1223+
free(ptr->name);
1224+
free(ptr);
1225+
}
12121226
break;
12131227
}
12141228
}
@@ -1413,11 +1427,17 @@ cppline {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
14131427
;
14141428
yytext[i+1] = '\0';
14151429

1416-
for (defptr = defines;
1417-
defptr != NULL &&
1418-
strcmp(yytext, defptr->olddef) != 0;
1419-
defptr = defptr->next)
1420-
/* skip */ ;
1430+
/* Does a definition exist? */
1431+
for (defptr = defines; defptr; defptr = defptr->next)
1432+
{
1433+
if (strcmp(yytext, defptr->name) == 0)
1434+
{
1435+
/* Found it, but is it currently undefined? */
1436+
if (defptr->value == NULL)
1437+
defptr = NULL; /* pretend it's not found */
1438+
break;
1439+
}
1440+
}
14211441

14221442
this_active = (defptr ? ifcond : !ifcond);
14231443
stacked_if_value[preproc_tos].active =
@@ -1438,7 +1458,7 @@ cppline {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
14381458
yyterminate();
14391459
}
14401460
<def_ident>{identifier} {
1441-
old = mm_strdup(yytext);
1461+
newdefsymbol = mm_strdup(yytext);
14421462
BEGIN(def);
14431463
startlit();
14441464
}
@@ -1447,26 +1467,31 @@ cppline {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
14471467
yyterminate();
14481468
}
14491469
<def>{space}*";" {
1450-
struct _defines *ptr, *this;
1470+
struct _defines *ptr;
14511471

1472+
/* Does it already exist? */
14521473
for (ptr = defines; ptr != NULL; ptr = ptr->next)
14531474
{
1454-
if (strcmp(old, ptr->olddef) == 0)
1455-
{
1456-
free(ptr->newdef);
1457-
ptr->newdef = mm_strdup(literalbuf);
1458-
}
1475+
if (strcmp(newdefsymbol, ptr->name) == 0)
1476+
{
1477+
free(ptr->value);
1478+
ptr->value = mm_strdup(literalbuf);
1479+
/* Don't leak newdefsymbol */
1480+
free(newdefsymbol);
1481+
break;
1482+
}
14591483
}
14601484
if (ptr == NULL)
14611485
{
1462-
this = (struct _defines *) mm_alloc(sizeof(struct _defines));
1463-
1464-
/* initial definition */
1465-
this->olddef = old;
1466-
this->newdef = mm_strdup(literalbuf);
1467-
this->next = defines;
1468-
this->used = NULL;
1469-
defines = this;
1486+
/* Not present, make a new entry */
1487+
ptr = (struct _defines *) mm_alloc(sizeof(struct _defines));
1488+
1489+
ptr->name = newdefsymbol;
1490+
ptr->value = mm_strdup(literalbuf);
1491+
ptr->cmdvalue = NULL;
1492+
ptr->used = NULL;
1493+
ptr->next = defines;
1494+
defines = ptr;
14701495
}
14711496

14721497
BEGIN(C);
@@ -1483,6 +1508,7 @@ cppline {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
14831508
<<EOF>> {
14841509
if (yy_buffer == NULL)
14851510
{
1511+
/* No more input */
14861512
if (preproc_tos > 0)
14871513
{
14881514
preproc_tos = 0;
@@ -1492,16 +1518,20 @@ cppline {space}*#([^i][A-Za-z]*|{if}|{ifdef}|{ifndef}|{import})((\/\*[^*/]*\*+
14921518
}
14931519
else
14941520
{
1521+
/* Revert to previous input source */
14951522
struct _yy_buffer *yb = yy_buffer;
14961523
int i;
14971524
struct _defines *ptr;
14981525

1526+
/* Check to see if we are exiting a macro value */
14991527
for (ptr = defines; ptr; ptr = ptr->next)
1528+
{
15001529
if (ptr->used == yy_buffer)
15011530
{
15021531
ptr->used = NULL;
1503-
break;
1532+
break; /* there can't be multiple matches */
15041533
}
1534+
}
15051535

15061536
if (yyin != NULL)
15071537
fclose(yyin);
@@ -1726,15 +1756,24 @@ ecpg_isspace(char ch)
17261756
return false;
17271757
}
17281758

1729-
static bool isdefine(void)
1759+
/*
1760+
* If yytext matches a define symbol, begin scanning the symbol's value
1761+
* and return true
1762+
*/
1763+
static bool
1764+
isdefine(void)
17301765
{
17311766
struct _defines *ptr;
17321767

17331768
/* is it a define? */
17341769
for (ptr = defines; ptr; ptr = ptr->next)
17351770
{
1736-
if (strcmp(yytext, ptr->olddef) == 0 && ptr->used == NULL)
1771+
/* notice we do not match anything being actively expanded */
1772+
if (strcmp(yytext, ptr->name) == 0 &&
1773+
ptr->value != NULL &&
1774+
ptr->used == NULL)
17371775
{
1776+
/* Save state associated with the current buffer */
17381777
struct _yy_buffer *yb;
17391778

17401779
yb = mm_alloc(sizeof(struct _yy_buffer));
@@ -1743,18 +1782,30 @@ static bool isdefine(void)
17431782
yb->lineno = yylineno;
17441783
yb->filename = mm_strdup(input_filename);
17451784
yb->next = yy_buffer;
1785+
yy_buffer = yb;
17461786

1747-
ptr->used = yy_buffer = yb;
1787+
/* Mark symbol as being actively expanded */
1788+
ptr->used = yb;
17481789

1749-
yy_scan_string(ptr->newdef);
1790+
/*
1791+
* We use yy_scan_string which will copy the value, so there's
1792+
* no need to worry about a possible undef happening while we
1793+
* are still scanning it.
1794+
*/
1795+
yy_scan_string(ptr->value);
17501796
return true;
17511797
}
17521798
}
17531799

17541800
return false;
17551801
}
17561802

1757-
static bool isinformixdefine(void)
1803+
/*
1804+
* Handle replacement of INFORMIX built-in defines. This works just
1805+
* like isdefine() except for the source of the string to scan.
1806+
*/
1807+
static bool
1808+
isinformixdefine(void)
17581809
{
17591810
const char *new = NULL;
17601811

0 commit comments

Comments
 (0)