Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

merge: 20231022 #441

Merged
merged 13 commits into from
Oct 22, 2023
Merged

merge: 20231022 #441

merged 13 commits into from
Oct 22, 2023

Conversation

mrdrivingduck
Copy link
Member

No description provided.

mrdrivingduck and others added 13 commits October 22, 2023 08:32
Reviewed-by: Abhijit Menon-Sen
Reviewed-by: Andres Freund
Discussion: https://postgr.es/m/CAApHDvobgmCs6CohqhKTUf7D8vffoZXQTCBTERo9gbOeZmvLTw%40mail.gmail.com
Backpatch-through: 11, where JIT was added
…functions.

Previously we only copied the function attributes. That caused problems at
least on s390x: Because we didn't copy the 'zeroext' attribute for
ExecAggTransReparent()'s *IsNull parameters, expressions invoking it didn't
ensure that the upper bytes of the registers were zeroed. In the - relatively
rare - cases where not, ExecAggTransReparent() wrongly ended up in the
newValueIsNull branch due to the register not being zero. Subsequently causing
a crash.

It's quite possible that this would cause problems on other platforms, and in
other places than just ExecAggTransReparent() on s390x.

Thanks to Christoph (and the Debian project) for providing me with access to a
s390x machine, allowing me to debug this.

Reported-By: Christoph Berg
Author: Andres Freund
Discussion: https://postgr.es/m/[email protected]
Backpatch: 11-, where JIT was added
Unfortunately in LLVM 3.9 LLVMGetAttributeCountAtIndex(func, index)
crashes when called with an index that has 0 attributes. Since there's
no way to work around this in the C API, add a small C++ wrapper doing
so.

The only reason this didn't fail before 7255943 is that there
always are function attributes...

Author: Andres Freund <[email protected]>
Discussion: https://postgr.es/m/[email protected]
Backpatch: 11-, like 7255943
As there haven't been problem on the buildfarm due to this change,
backpatch 6c57f2e now.

Author: Andres Freund
Discussion: https://postgr.es/m/[email protected]
Backpatch: 11-, where jit support was added
…ions.

clang only uses the 'i1' type for scalar booleans, not for pointers to
booleans (as the pointer might be pointing into a larger memory
allocation). Therefore a pointer-to-bool needs to the "storage" boolean.

There's no known case of wrong code generation due to this, but it seems quite
possible that it could cause problems (see e.g. 7255943).

Author: Andres Freund
Discussion: https://postgr.es/m/[email protected]
Backpatch: 11-, where jit support was added
LLVM 13 (due out in September) has changed the semantics of
LLVMOrcAbsoluteSymbols(), so we need to bump some reference counts to
avoid a double-free that causes crashes and bad query results.

A proactive change seems necessary to avoid having a window of time
where our respective latest releases would interact badly.  It's
possible that the situation could change before then, though.

Thanks to Fabien Coelho for monitoring bleeding edge LLVM and Andres
Freund for tracking down the change.

Back-patch to 11, where the JIT code arrived.

Discussion: https://postgr.es/m/CA%2BhUKGLEy8mgtN7BNp0ooFAjUedDTJj5dME7NxLU-m91b85siA%40mail.gmail.com
Code inlined by LLVM can crash or fail with "Relocation type not
implemented yet!" if it tries to access thread local variables.  Don't
inline such code.

Back-patch to 11, where LLVM arrived.  Bug #16696.

Author: Dmitry Marakasov <[email protected]>
Reviewed-by: Andres Freund <[email protected]>
Discussion: https://postgr.es/m/[email protected]
…ors.

If an allocation failed within LLVM it is not safe to call back into LLVM as
LLVM is not generally safe against exceptions / stack-unwinding. Thus errors
while in LLVM code are promoted to FATAL. However llvm_shutdown() did call
back into LLVM even in such cases, while llvm_release_context() was careful
not to do so.

We cannot generally skip shutting down LLVM, as that can break profiling. But
it's OK to do so if there was an error from within LLVM.

Reported-By: Jelte Fennema <[email protected]>
Author: Andres Freund <[email protected]>
Author: Justin Pryzby <[email protected]>
Discussion: https://postgr.es/m/AM5PR83MB0178C52CCA0A8DEA0207DC14F7FF9@AM5PR83MB0178.EURPRD83.prod.outlook.com
Backpatch: 11-, where jit was introduced
Since LLVM 14 has stopped changing and is about to be released,
back-patch the following changes from the master branch:

  e6a7600
  807fee1
  a56e7b6

Back-patch to 11, where LLVM JIT support came in.
Per https://llvm.org/docs/OpaquePointers.html, support for non-opaque
pointers still exists and we can request that on our context.  We have
until LLVM 16 to move to opaque pointers, a much larger change.

Back-patch to 11, where LLVM support arrived.

Author: Thomas Munro <[email protected]>
Author: Andres Freund <[email protected]>
Discussion: https://postgr.es/m/CAMHz58Sf_xncdyqsekoVsNeKcruKootLtVH6cYXVhhUR1oKPCg%40mail.gmail.com
While on it, newlines are removed from the end of two elog() strings.
The others are simple grammar mistakes.  One comment in pg_upgrade
referred incorrectly to sequences since a7e5457.

Author: Justin Pryzby
Discussion: https://postgr.es/m/[email protected]
Backpatch-through: 11
llvm_release_context() called llvm_enter_fatal_on_oom(), but was missing
the corresponding llvm_leave_fatal_on_oom() call. As a result, if JIT was
used at all, we were almost always in the "fatal-on-oom" state.

It only makes a difference if you use an extension written in C++, and
run out of memory in a C++ 'new' call. In that case, you would get a
PostgreSQL FATAL error, instead of the default behavior of throwing a
C++ exception.

Back-patch to all supported versions.

Reviewed-by: Daniel Gustafsson
Discussion: https://www.postgresql.org/message-id/[email protected]
@polardb-bot
Copy link

polardb-bot bot commented Oct 22, 2023

Hi @mrdrivingduck ~ Thanks for your contribution in this PR. ❤️

Please make sure that your PR conforms the standard, and has passed all the checks.

We will review your PR as soon as possible.

@CLAassistant
Copy link

CLAassistant commented Oct 22, 2023

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 6 committers have signed the CLA.

✅ mrdrivingduck
❌ david-rowley
❌ macdice
❌ michaelpq
❌ hlinnaka
❌ anarazel
You have signed the CLA already but the status is still pending? Let us recheck it.

@mrdrivingduck mrdrivingduck merged commit 45933cb into POLARDB_11_STABLE Oct 22, 2023
6 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants