Neil Conway <neilc@samurai.com> writes:
> I think you're missing the point. Obviously the current code is wrong,
> the debate is over the best way to fix it. Jeroen's interpretation of
> the spec suggests that merely having libpq_gettext() preserve errno is
> not sufficient. I'm not convinced this his interpretation is correct,
> but it is a question worth resolving.

(1) The fact that gettext works at all seems to me to be sufficient
empirical evidence that it's a working solution. (2) Whether there are
sequence points in the function call or not, there most certainly are
sequence points inside each called function. The spec allows the
functions involved to be called in an unspecified order, but it doesn't
allow the compiler to interleave the execution of several functions.
(3) Interpretation or not, the approach that Jeroen proposes is
unmaintainable; people will not remember to use such a kluge everywhere
they'd need to. I'd much rather fix it in one place and do whatever we
have to do to keep the compiler from breaking that one place.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 7: don't forget to increase your free space map settings

Neil Conway wrote:
> Tom Lane wrote:
>> I think this is all irrelevant language-lawyering; jtv spotted the true
>> problem which is that we do not protect errno during the *first* call of
>> libpq_gettext.
>
> I think you're missing the point. Obviously the current code is wrong,
> the debate is over the best way to fix it. Jeroen's interpretation of
> the spec suggests that merely having libpq_gettext() preserve errno is
> not sufficient. I'm not convinced this his interpretation is correct,
> but it is a question worth resolving.

Agree totally. If my interpretation is wrong then I'll happily get on
with my life and let everyone else do the same. I was at that point once
already, but Neil took another close look at the relevant part of the C
standard he dug up and found this potential problem.

I really don't like playing the smart-alec language lawyer here, but I've
been following compiler developments and they are moving in a direction
that makes this relevant. I do want to be sure that we're shipping
correct code, not just code that practically speaking suppresses the
symptoms of its bugs for a while, on most compilers, for the most popular
CPU architectures.

Moreover, I don't want to go through all of this again when the regression
occurs and we think we've solved it forever and the problem must be
somewhere else. I've been losing enough sleep over what I thought must be
bugs in libpqxx that I just couldn't put my finger on.


Jeroen



---------------------------(end of broadcast)---------------------------
TIP 7: don't forget to increase your free space map settings

Tom Lane wrote:
>
> (1) The fact that gettext works at all seems to me to be sufficient
> empirical evidence that it's a working solution.

Tom, you have GOT to be joking. I agree with some of the things you say
(see below), but here you're just not making sense. Consider:

1. We're only talking about a very specific property of gettext(), namely
restoration of errno in calls that are not ordered w.r.t. a use of errno.
This is only relevant in a very limited number of all calls to gettext(),
so the mere fact that gettext() works doesn't prove anything relevant.

2. In those limited few cases, and for this specific property, we already
know that something in this general area is *not* working, so you'd have
to find, fix and test that before you could even make this argument.

3. Given that a call to gettext() crosses not just object-file but actual
library boundaries, what do you think the chances are--regardless of
language garantees and compiler aggressiveness--that we'd see the compiler
interleaving instructions from gettext() with a use of errno on the other
side of that boundary? Does the assumption that gettext() works at all,
therefore, make a reliable indication that there is no problem?

4. In the case of libpq_gettext(), we're not crossing library boundaries.
We're not even crossing object-file boundaries in some cases. That makes
the chances of instructions being scheduled across a call much
greater--and the question about sequence points much more relevant.

5. From what I understand, the latest versions of gcc are actually
beginning to schedule instructions across function call boundaries. That
will undoubtedly increase in the future. There is even a new feature that
can, as far as I can see, lead to instruction scheduling across source
file boundaries.


> (2) Whether there are
> sequence points in the function call or not, there most certainly are
> sequence points inside each called function. The spec allows the
> functions involved to be called in an unspecified order, but it doesn't
> allow the compiler to interleave the execution of several functions.

That would answer the big question here, but where does it say that? I
saw Neil's point that the sequence points before function calls apply for
the nested calls as well as the outer one, but there is no ordering
between those "nested-call" sequence points. It's all easy when you have
a total ordering, but we're in a partial ordering here.

So the missing piece of the puzzle is still that same question. Obviously
the compiler isn't allowed to interleave function executions (or at least
not let you find out about it) if there is a sequence point _between_
them. There is in sequential calls, for example, because there is a
sequence point before the function is entered. But what happens if there
isn't a separating sequence point, like here?


> (3) Interpretation or not, the approach that Jeroen proposes is
> unmaintainable; people will not remember to use such a kluge everywhere
> they'd need to. I'd much rather fix it in one place and do whatever we
> have to do to keep the compiler from breaking that one place.

That means you haven't seen the approach I suggested the day before
yesterday, where I also explained that I felt it best to fix the incumbent
bugs first before refactoring the result. You're still talking about my
initial quick-fix patch, which IMHO could have gone in already while we
were arguing over a long-term solution. That patch was ugly solely in the
sense that the original broken code was ugly; if you want to use words
like "kluge" then please direct them there. The copy-paste style of
writing loops didn't help either.

As I suggested on Wednesday, maybe we can wrap the combination of
printfPQExpBuffer() and libpq_gettext() into a single function that takes
a PGconn *, a format string, and varargs. This makes the calls a lot
shorter and simpler, it moots the question of sequence points because
errno would be the first thing to be evaluated, and en passant we'll fix a
few cases where we forgot to call libpq_gettext(). We'd have to have a
similar wrapper for snprintf(), but then I think all cases in libpq are
covered.


Jeroen



---------------------------(end of broadcast)---------------------------
TIP 5: don't forget to increase your free space map settings

jtv@xs4all.nl writes:
> That would answer the big question here, but where does it say that? I
> saw Neil's point that the sequence points before function calls apply for
> the nested calls as well as the outer one, but there is no ordering
> between those "nested-call" sequence points. It's all easy when you have
> a total ordering, but we're in a partial ordering here.

This is utter nonsense. If the sequence points within a function do not
follow (in an execution-order sense) the one at the call site, then no C
program on the planet will manage to work.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 5: don't forget to increase your free space map settings

jtv@xs4all.nl writes:
> That would answer the big question here, but where does it say that?

Also, if you really insist on an authoritative statement, try this text
(from Annex D of the C99 draft standard, "Formal model of sequence
points"):

D.2.2 Function calls |

[#1] Where an expression involves a function call, that call |
is ``atomic'' for the purposes of this model. There must be |
a sequence point immediately before (see 6.5.2.2) and after |
each function call (either because it ends in a full |
expression, or because it is required by 7.1.4), and so it |
can be seen that the effects of the call -- for the |
purposes of the surrounding expression -- can be determined |
purely by the read and write events involved in it, ignoring |
their ordering. These events cannot be interspersed with |
events from outside the call. Therefore this model treats a |
function call as being a sequence point. |

Cross-function optimization by the compiler is all well and dandy,
but it is on the compiler's head to make sure it doesn't change
the observable semantics of the program. Not ours.

regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

http://archives.postgresql.org

Tom Lane wrote:
>> That would answer the big question here, but where does it say that? I
>> saw Neil's point that the sequence points before function calls apply
>> for
>> the nested calls as well as the outer one, but there is no ordering
>> between those "nested-call" sequence points. It's all easy when you
>> have
>> a total ordering, but we're in a partial ordering here.
>
> This is utter nonsense. If the sequence points within a function do not
> follow (in an execution-order sense) the one at the call site, then no C
> program on the planet will manage to work.

That's not what I said though. In retrospect you could interpret it that
way, but what would be the point in choosing an obviously nonsensical
interpretation? Yes, the sequence points within a function obviously
follow the function point marking the call itself. But that's beside the
issue. It's already implicitly assumed in everything we say here, as I'm
sure you're aware. When I say 'those "nested-call" sequence points' I
refer to the two sequence points marking the two respective nested calls.

Here's the problem again: we have two calls to respective functions a()
and b(), each call of course marked by a sequence point (call them a0 and
b0) and each function comprising a string of sequence points internally
(say a1a2..an and b1b2..bm) whose internal order must be respected. No
problem so far, I hope. a1a2 etc. must be performed in that order and
b1b2 etc. must be performed in that order. But no ordering is imposed
between a0 and b0!

Now, if there is a requirement somewhere in the standard that says, like
you say, that the execution of two function calls may not be interleaved,
then we're in the clear. But I didn't find anything to say that. Absent
that guarantee AFAICS the compiler is allowed to pick any interleaving of
a() and b() that respects their respective strings of sequence points:
a0b0a1b1a2b2 would do, or b0b1a0a1b2a2, and so on. If that is the case,
then your proposed solution may start to fail mysteriously as compilers
progress.

And now that we're on the subject, shouldn't the default cases of the SSL
error-code switches in pqsecure_read() and pqsecure_write()
(src/interfaces/libpq/fe-secure.c) restore errno as well?


Jeroen



---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

http://archives.postgresql.org

Tom Lane wrote:

>> That would answer the big question here, but where does it say that?
>
> Also, if you really insist on an authoritative statement, try this text
> (from Annex D of the C99 draft standard, "Formal model of sequence
> points"):

Thank you, that would answer the question. There is no "also" about it;
it's exactly what I was asking all along. The conclusive answer for us
would be in the C89 standard of course, where (at least in the draft that
Neil quoted) I haven't been able to find anything like this. :-(


Jeroen



---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

jtv@xs4all.nl wrote:
> Thank you, that would answer the question. There is no "also" about it;
> it's exactly what I was asking all along. The conclusive answer for us
> would be in the C89 standard of course, where (at least in the draft that
> Neil quoted) I haven't been able to find anything like this. :-(

If in the future when compilers actually do begin applying aggressive
enough optimization that this might be a problem in practice, it seems
likely they will also have updated to C99. It seems a little much to
imagine (a) a compiler that does this in the first place (b) a compiler
that varies its interpretation of sequence points in an extremely subtle
way depending on the dialect of C in use. IOW, I think C99 is sufficient.

-Neil

---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match

On Sat, 9 Jul 2005 jtv@xs4all.nl wrote:

> Tom Lane wrote:
>
> >> That would answer the big question here, but where does it say that?
> >
> > Also, if you really insist on an authoritative statement, try this text
> > (from Annex D of the C99 draft standard, "Formal model of sequence
> > points"):
>
> Thank you, that would answer the question. There is no "also" about it;
> it's exactly what I was asking all along. The conclusive answer for us
> would be in the C89 standard of course, where (at least in the draft that
> Neil quoted) I haven't been able to find anything like this. :-(

I believe overlap of functions in the same expression was disallowed by
the response to defect report 087. The only reference I've been able to
find right now (since the committee seems to have removed the C89 DRs from
their site) is in the response to DR 287 which includes:

"Proposed Committee Response
As noted in the response to DR 087, function calls in the same expression
do not overlap. This has not changed for C99."

---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match

Stephan Szabo wrote:

> I believe overlap of functions in the same expression was disallowed by
> the response to defect report 087. The only reference I've been able to
> find right now (since the committee seems to have removed the C89 DRs from
> their site) is in the response to DR 287 which includes:
>
> "Proposed Committee Response
> As noted in the response to DR 087, function calls in the same expression
> do not overlap. This has not changed for C99."

Yay! Good one. The quote Tom gave was not in the C99 draft I have access
to, and the 1998 C++ standard only contains a footnote implying that the C
standard at the time _did_ allow overlap of function calls whose order is
not specified. But what you quote here (apart from the "Proposed")
implies that C89 makes the guarantees we need.


Jeroen



---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly