Closed dmabupt closed 2 years ago
This doesn't seem to have been fixed. Looks like it's coming from these two symbols of db2ia.node:
dump -Tv -X64 db2ia.node | grep ImpExp
[68] 0x00000000 undef ImpExp DS EXTref . _ZN9__gnu_cxx12__to_xstringINSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEcEET_PFiPT0_mPKS8_PcEmSB_z
[69] 0x00000000 undef ImpExp DS EXTref . _ZNSt6vectorIcSaIcEE12emplace_backIJcEEEvDpOT_
According to demangler.com, the symbols are
std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > __gnu_cxx::__to_xstring<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, char>(int (*)(char*, unsigned long, char const*, char*), unsigned long, char const*, ...)
void std::vector<char, std::allocator<char> >::emplace_back<char>(char&&)
For some reason, these symbols are being imported and re-exported from the main executable by db2ia.node. This works because our node8 - node14 builds built with GCC export these as weak symbols:
[18949] 0x200f9360 .data wEXP DS SECdef [noIMid] _ZN9__gnu_cxx12__to_xstringINSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEcEET_PFiPT0_mPKS8_PcEmSB_z
[20348] 0x20100c80 .data wEXP DS SECdef [noIMid] _ZNSt6vectorIcSaIcEE12emplace_backIJcEEEvDpOT_
However, node16 - built with GCC 10 - does not.
Yes, I placed a wrong tarball. When I use the real gcc10 built db2i.node file, the node10 app fails --
$ node index.js
Server running on port 8081!
Error: rtld: 0712-001 Symbol _ZNSt6vectorIcSaIcEE17_M_realloc_insertIJcEEEvN9__gnu_cxx17__normal_iteratorIPcS1_EEDpOT_ was referenced
from module /home/xumeng/git/ibmi-oss-examples/nodejs/mynodeapp/node_modules/idb-connector/lib/binding/Release/napi3-ibmi-ppc64/db2ia.node(), but a runtime definition
of the symbol was not found.
rtld: 0712-001 Symbol _ZNSt7__cxx119to_stringEi was referenced
from module /home/xumeng/git/ibmi-oss-examples/nodejs/mynodeapp/node_modules/idb-connector/lib/binding/Release/napi3-ibmi-ppc64/db2ia.node(), but a runtime definition
of the symbol was not found.
rtld: 0712-001 Symbol _ZZNSt8__detail18__to_chars_10_implIjEEvPcjT_E8__digits was referenced
from module /home/xumeng/git/ibmi-oss-examples/nodejs/mynodeapp/node_modules/idb-connector/lib/binding/Release/napi3-ibmi-ppc64/db2ia.node(), but a runtime definition
of the symbol was not found.
rtld: 0712-002 fatal error: exiting.
@kadler ,
I checked the function emplace_back
and found it existed only in GCC 6.3 and deleted in 10.2.
I do not know why db2ia.node refer to this deleted function? Maybe we need to upgrade V8 or node API?
Maybe coming from node-addon-api? I'm not sure why Node < 16 is exporting those symbols, nor why the shared library is built to expect them from the main program.
The libstdc++ library from gcc10 exports these emplace_back stuff as weak symbols, while gcc6 does not export them at all.
$ dump -Tv -X64 shr_64.o | grep emplace_back
[6470] 0x2003d978 .data wEXP DS SECdef [noIMid] _ZNSt5dequeINSt10filesystem4_DirESaIS1_EE12emplace_backIJS1_EEERS1_DpOT_
[6546] 0x2003e080 .data wEXP DS SECdef [noIMid] _ZNSt5dequeINSt10filesystem4pathESaIS1_EE12emplace_backIJS1_EEERS1_DpOT_
[6692] 0x2003ed88 .data wEXP DS SECdef [noIMid] _ZNSt5dequeINSt10filesystem7__cxx114_DirESaIS2_EE12emplace_backIJS2_EEERS2_DpOT_
I am opening an issue against the N-API project. Since we built this against the N-API, it shouldn't need to get rebuilt for newer Node.js versions. It should be binary compatible (that's the point of N-API, or at least one of the more important objectives). This could be an N-API defect, or they might at least be able to provide guidance.
We discussed in the Node-api team meeting today. From the quick search it looks like the node-addon-api does not use emplace-back so we believe it would be specific to the addon versus something introduced by node-addon-api itself.
There seems to be some confusion
Is it that they export different symbols ?
It also sounds like it is symbols from Node.js which are outside of the node-api which are being used by the addon that are the issue. Is that right ?
One way to validate may be to create a simple addon that uses std::vector::emplace_back. Building it with gcc 6 and then running with gcc 10 in the path, it should fail with the same error, if we remove the use in the native addon it should then pass.
We looked at linux and on Node 10.x and Node 12.x those symbols are not exported so this may be an IBM i specific issue.
There seems to be some confusion
* [#141 (comment)](https://github.com/IBM/nodejs-idb-connector/issues/141#issuecomment-828097520) says that emplace back existing in gcc 6 but not in 10, * [#141 (comment)](https://github.com/IBM/nodejs-idb-connector/issues/141#issuecomment-836257605) seems to say that emplace back symbols exist in gcc 10 but not 6
When built with GCC 6, the node binary exports these two symbols which are not exported from GCC 6 libstdc++ When built with GCC 10, the node binary does not export these two symbols, but are exported from GCC 10 libstdc++
I don't see any references to emplace_back in the the idb-connector code, however we do use std::vector, so it's possible one of the std::vector methods we do use is an inline function wrapper around emplace_back.
The symbol is coming from /QOpenSys/pkgs/lib/nodejs14/include/node/node.exp, along with a lot of other libstdc++ symbols.
When I commented them out in the exp file, the build succeeded with db2ia.node now exporting those as weak symbols (just as the node binary does). That build of db2ia.node successfully loaded and retrieved data using our node16.
I think for now, we can fix up our node < 16 rpms to remove those two symbols from the node.exp. We might need to look and see if there's any other symbols that need to be removed as well or why they end up there in the first place.
I seemed to remember node.exp from build steps needs specifically for AIX.
I can see in the Makefile
ifeq ($(OSTYPE),aix)
DOCBUILDSTAMP_PREREQS := $(DOCBUILDSTAMP_PREREQS) out/$(BUILDTYPE)/node.exp
endif
If I remember correctly the problem was that AIX by default exports no symbols and the node.exp file was used to generate the symbols that should be exported. It's quite possible that it's exporting too much.
From the node.gyp file
[ 'node_shared=="true"', {
'node_target_type%': 'shared_library',
'conditions': [
['OS=="aix"', {
# For AIX, always generate static library first,
# It needs an extra step to generate exp and
# then use both static lib and exp to create
# shared lib.
'node_intermediate_lib_type': 'static_library',
}, {
'node_intermediate_lib_type': 'shared_library',
}],
],
We'd probably need some AIX/IBM i suggestions on how to better build the exp file, limiting it to just symbols that should be exported.
I don't quite remember how the exp file is generated and I don't see a specific step in the Node gyp or Makefile. It may be a by product generating the shared library first and then using that in the executable.
I wonder if there is an easy way to filter based on an exp for the C++ library ?
I have released idb-connector v1.2.13 which is built with the patched Node12 and it works with Node16 well as far as I can see.
@dmabupt, @kadler any thoughts on changing how AIX/ibm i is built to address the overall issue behind this?
@mhdawson I think the problem is somewhere in https://github.com/nodejs/node/blob/master/tools/create_expfile.sh
I'm running a build now to try and see what gets pulled in to this script and where the symbols are coming from. We could try adjusting the awk script to disable exporting weak symbols, though I'm not sure if that will affect anything else.
FYI, I downloaded the latest AIX node 14 binary and it is also affected by this issue, though I don't know what version of GCC is used for the official builds. I'll open an issue once I have more details.
Disabling weak symbols causes ~13k fewer symbols to be exported in Node 10. Dumping the node14 binary, I see over 15k weak symbols (dump -X64 -Tv /QOpenSys/pkgs/lib/nodejs14/bin/node | grep wEXP | wc -l
).
I also found that _ZNSt6vectorIcSaIcEE12emplace_backIJcEEEvDpOT_
is exported from the Linux binary:
[kadler@kadler node-v14.17.0-linux-x64]$ nm -D bin/node | grep _ZNSt6vectorIcSaIcEE12emplace_backIJcEEEvDpOT_
0000000000b85a30 W _ZNSt6vectorIcSaIcEE12emplace_backIJcEEEvDpOT_
Additionally, over 11k weak symbols are exported from the Linux node binary, so we should probably export them too. I'm not sure how we'd slice just these specific symbols in a general way.
@kadler thanks for the updates. From https://github.com/nodejs/node/blob/master/BUILDING.md I think it is gcc 8 that we are using on AIX. This line in select-compiler.sh would seem to confirm that: https://github.com/nodejs/build/blob/1b379cbb266bb567a8c94406de6153b88572220f/jenkins/scripts/select-compiler.sh#L106
Any progress on this issue? We at @HolterDev planned to upgrade to node.js 16 but are running into the problem described here.
Has this been fixed with a new version of idb-connector
? It seems like we installed an older version of the package through idb-pconnector
, even though its current package.json sets idb-connector^1.2.15 as dependency. See https://github.com/IBM/nodejs-idb-pconnector/blob/master/package.json
Looks like the npm registry has an outdated version of dependency sets... see https://registry.npmjs.org/idb-pconnector
patrick@testas:/node/NodeCode-HRAS$ npm list idb-connector
node-code@1.0.0 /node/NodeCode-HRAS
`-- ho-framework@1.0.0 -> ./packages/ho-framework
`-- idb-pconnector@1.0.8
`-- idb-connector@1.2.9
@dmabupt can you answer the question from @patrickhrastnik
AFAIK, I merged a change for our node 10, 12, and 14 rpms a year ago, so this should be fixed. Unfortunately, I forgot to update this issue with the resolution.
We ended up adding some code to our rpm spec files to remove the offending symbols from the node.exp. I'm not sure the correct way to do that in case issues arise in the future, but with Node.js 16 built on GCC 10 we have not seen it add these erroneous entries.
So if our Node.js packages are supposedly fixed, why are we having issues yet? Sounds like idb-pconnector is still pulling in the 2-year old idb-connector which has the problem. @abmusse it looks like idb-pconnector master has updated to 1.2.15, but it hasn't been pushed to npm yet. Can you push out a new release?
@kadler I Published v1.1.0 of idb-pconnector please @patrickhrastnik give this a shot
Thanks @abmusse
Server starts now, but upon calling an Sql stored procedure via idb-pconnector/DBPool/prepareExecute
the following warning shows up in the console:
Statement deprecated As of 1.1.0, bindParam() is deprecated and you should use bindParameters() instead. node_modules/idb-pconnector/lib/dbPool.js:272:23
and under the hood the error described in
occurs with a statement properly working with the previous implementation using idb-pconnector@1.0.8
.
Installed package versions for test:
patrick@testas:/node/NodeCode-HRAS$ node --version
v16.11.1
patrick@testas:/node/NodeCode-HRAS$ npm --version
8.0.0
patrick@testas:/node/NodeCode-HRAS$ npm list idb-connector
node-code@1.0.0 /node/NodeCode-HRAS
`-- idb-pconnector@1.1.0
`-- idb-connector@1.2.16
To make the report complete, here's the log object with stack information I got. It's preprocessed through our log server and the error message is in German, but it's SQLCODE=-191
.
{
"timestamp": "2022-06-14T13:17:17.962Z",
"log.level": "Error",
"message": "Error executing XML-request",
"metaObject": {},
"error.stack": "Error: PE: Failed to execute() statement\n at /node/NodeCode-HRAS/node_modules/idb-pconnector/lib/dbPool.js:283:18\n at async DBPool.prepareExecute (/node/NodeCode-HRAS/node_modules/idb-pconnector/lib/dbPool.js:281:17)\nCaused By:\n Error: SQLSTATE=22504 SQLCODE=-191 Mischbytedaten oder UTF-8-Daten haben falsches Format.\n at Object.<anonymous> (/node/NodeCode-HRAS/node_modules/idb-pconnector/lib/statement.js:193:18)",
"error.message": "PE: Failed to execute() statement",
"agent.port": 8085,
"agent.environment": "Dev",
"agent.name": "testas.ad.holter.at",
"type": "nodecodelog"
}
Does idb-pconnector
need an implementation update to use the new function bindParameters
instead of bindParam
, or is the old way still supported with a warning? I guess if so I should open a new issue at idb-pconnector
.
Further implementation details regarding my last message because they might be necessary for further evaluation
Here's the TypeScript code piece calling the according function in idb-pconnector
:
idbPConnectionPool
.prepareExecute(sqlStatement, sqlParams, {
io: "both",
})
.then((result) => {
// some code
})
.catch((reason) => {
Logger.warn("Error executing a stored procedure", reason);
reject(reason);
});
Here's some live data from a debug session
sqlStatement
'CALL HBUHA.XML01(?, ?, ?, ?, ?)'
sqlParams
(5) ['XCHKONLANM', 0, '', 24695, '<snd><data><kennwort>redacted</kennwort><username>patrick</username></data></snd>']
This is the interface definition of XML01
dcl-pi *n;
prog char(20);
fnr packed(5);
ftext char(100);
depn packed(7);
xmldaten varchar(32000);
traceId char(36) options(*nopass);
runAsUserId char(10) options(*nopass);
end-pi;
@patrickhrastnik the old bindParam function still works, but is deprecated. The latest idb-pconnector package has bindParameters function, but you would need to change your code to use it instead of bindParam.
Is there an SQL procedure definition for XML01 and if so, what is it? Are there any inout or out parameters?
Pardon @kadler I'm not sure I'm getting this correctly
https://github.com/IBM/nodejs-idb-pconnector/blob/master/lib/dbPool.js seems to still use the old function bindParam
Nevertheless, if it still works it's fine I guess
This is the Sql procedure declaration code for XML01. All parameters are specified to be INOUT
.
CREATE PROCEDURE HBUHA.XML01 (
INOUT PROGRAMM CHAR(20) ,
INOUT FEHLERNUMMER DECIMAL(5, 0) ,
INOUT FEHLERTEXT CHAR(100) ,
INOUT PROGRAMMNUMMER DECIMAL(7, 0) ,
INOUT XMLDATEN VARCHAR(32000) ,
INOUT TRACEID CHAR(36) DEFAULT '' ,
INOUT RUNASUSERID CHAR(10) DEFAULT '' )
LANGUAGE RPGLE
SPECIFIC HBUHA.XML01
NOT DETERMINISTIC
MODIFIES SQL DATA
CALLED ON NULL INPUT
EXTERNAL NAME 'HBUHA/XML01'
PARAMETER STYLE SQL ;
Ahh. @abmusse I thought you mentioned to me that there was a PR to make the pool API to use bindParameters. Did that get merged for 1.1.0? Or am I thinking about something different that changed in the pool API?
@kadler
pool API For prepareExecute currently does not use bindParameters. What got merged was PR to to configrue enableNumericTypeConversion
for the statements created in the pool.
Thanks for the memory refresh.
@patrickhrastnik I I'm gonna close this issue out since the UTF-8 problem you're running in to seems to be a separate issue.
Let's focus on identifying and fixing the UTF-8 issue under #147.
Describe the bug
To Reproduce Run any idb-connector app via Node.js 16.x