Skip to content

Commit a911775

Browse files
msimacekmsimacek
committed
[GR-58124] Fix leaking nulls to python in OSError.args
PullRequest: graalpython/3480
2 parents a6309ca + 2404d95 commit a911775

8 files changed

Lines changed: 68 additions & 48 deletions

File tree

‎graalpython/com.oracle.graal.python.test/src/tests/test_posix.py‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
3737
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
3838
# SOFTWARE.
39+
import errno
3940

4041
try:
4142
__graalpython__.posix_module_backend()
@@ -883,8 +884,13 @@ def test_sysconf(self):
883884
os.sysconf(object())
884885
with self.assertRaisesRegex(ValueError, 'unrecognized'):
885886
os.sysconf("nonexistent")
886-
with self.assertRaisesRegex(OSError, "Invalid argument"):
887+
try:
887888
os.sysconf(123456)
889+
except OSError as e:
890+
# We used to have a bug that the args would contain Java null for the filename in the args
891+
assert e.args == (errno.EINVAL, "Invalid argument")
892+
else:
893+
assert False
888894

889895

890896
if __name__ == '__main__':

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/cext/PythonCextBuiltins.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1762,7 +1762,7 @@ byte readBufferByte(long idx,
17621762
try {
17631763
return (posixSupportLib.mmapReadByte(PythonContext.get(posixSupportLib).getPosixSupport(), delegate.getPosixSupportHandle(), idx));
17641764
} catch (PosixException e) {
1765-
throw constructAndRaiseNode.get(inliningTarget).raiseOSError(null, e.getErrorCode(), e.getMessageAsTruffleString(), null, null);
1765+
throw constructAndRaiseNode.get(inliningTarget).raiseOSError(null, e.getErrorCode(), e.getMessageAsTruffleString());
17661766
}
17671767
}
17681768

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/multiprocessing/MultiprocessingGraalPyModuleBuiltins.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2019, 2023, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* The Universal Permissive License (UPL), Version 1.0
@@ -306,7 +306,7 @@ Object doWrite(int fd, PBytes data,
306306
throw PRaiseNode.raiseUncached(this, OSError, ErrorMessages.BAD_FILE_DESCRIPTOR);
307307
},
308308
() -> {
309-
throw PConstructAndRaiseNode.getUncached().raiseOSError(null, OSErrorEnum.EPIPE.getNumber(), OSErrorEnum.EPIPE.getMessage(), null);
309+
throw PConstructAndRaiseNode.getUncached().raiseOSError(null, OSErrorEnum.EPIPE);
310310
});
311311
return bytes.length;
312312
} finally {

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/exception/PBaseException.java‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -247,14 +247,21 @@ public String getFormattedMessage() {
247247
} else {
248248
return typeName;
249249
}
250-
} else if (args.getSequenceStorage().length() == 0) {
251-
return typeName;
252-
} else if (args.getSequenceStorage().length() == 1) {
253-
SequenceStorage store = args.getSequenceStorage();
254-
Object item = GetItemScalarNode.executeUncached(store, 0);
255-
return typeName + ": " + item.toString();
256250
} else {
257-
return typeName + ": " + args.toString();
251+
SequenceStorage storage = args.getSequenceStorage();
252+
if (storage.length() == 0) {
253+
return typeName;
254+
} else {
255+
StringBuilder builder = new StringBuilder(typeName);
256+
builder.append(": ");
257+
for (int i = 0; i < storage.length(); i++) {
258+
if (i > 0) {
259+
builder.append(", ");
260+
}
261+
builder.append(GetItemScalarNode.executeUncached(storage, i));
262+
}
263+
return builder.toString();
264+
}
258265
}
259266
}
260267

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/mmap/PMMap.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ byte readByte(int byteOffset,
146146
return posixLib.mmapReadByte(PythonContext.get(raiseNode).getPosixSupport(), getPosixSupportHandle(), byteOffset);
147147
} catch (PosixException e) {
148148
// TODO(fa) how to handle?
149-
throw raiseNode.get(inliningTarget).raiseOSError(null, e.getErrorCode(), fromJavaStringNode.execute(e.getMessage(), TS_ENCODING), null, null);
149+
throw raiseNode.get(inliningTarget).raiseOSError(null, e.getErrorCode(), fromJavaStringNode.execute(e.getMessage(), TS_ENCODING));
150150
}
151151
}
152152

@@ -160,7 +160,7 @@ void writeByte(int byteOffset, byte value,
160160
posixLib.mmapWriteByte(PythonContext.get(raiseNode).getPosixSupport(), getPosixSupportHandle(), byteOffset, value);
161161
} catch (PosixException e) {
162162
// TODO(fa) how to handle?
163-
throw raiseNode.get(inliningTarget).raiseOSError(null, e.getErrorCode(), fromJavaStringNode.execute(e.getMessage(), TS_ENCODING), null, null);
163+
throw raiseNode.get(inliningTarget).raiseOSError(null, e.getErrorCode(), fromJavaStringNode.execute(e.getMessage(), TS_ENCODING));
164164
}
165165
}
166166

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/socket/SocketNodes.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2021, 2023, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2021, 2024, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* The Universal Permissive License (UPL), Version 1.0
@@ -428,7 +428,7 @@ static Object makeSockAddr(VirtualFrame frame, Node inliningTarget, UniversalSoc
428428
throw raiseNode.get(inliningTarget).raise(NotImplementedError, toTruffleStringUncached("makesockaddr: unknown address family"));
429429
}
430430
} catch (PosixException e) {
431-
throw constructAndRaiseNode.get(inliningTarget).raiseOSError(frame, e.getErrorCode(), fromJavaStringNode.execute(e.getMessage(), TS_ENCODING), null, null);
431+
throw constructAndRaiseNode.get(inliningTarget).raiseOSError(frame, e.getErrorCode(), fromJavaStringNode.execute(e.getMessage(), TS_ENCODING));
432432
}
433433
}
434434

@@ -473,7 +473,7 @@ static Object makeAddr(VirtualFrame frame, Node inliningTarget, UniversalSockAdd
473473
throw raiseNode.get(inliningTarget).raise(NotImplementedError, toTruffleStringUncached("makesockaddr: unknown address family"));
474474
}
475475
} catch (PosixException e) {
476-
throw constructAndRaiseNode.get(inliningTarget).raiseOSError(frame, e.getErrorCode(), fromJavaStringNode.execute(e.getMessage(), TS_ENCODING), null, null);
476+
throw constructAndRaiseNode.get(inliningTarget).raiseOSError(frame, e.getErrorCode(), fromJavaStringNode.execute(e.getMessage(), TS_ENCODING));
477477
}
478478
}
479479
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/ssl/SSLOperationNode.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2021, 2023, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2021, 2024, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* The Universal Permissive License (UPL), Version 1.0
@@ -243,7 +243,7 @@ static void doSocket(VirtualFrame frame, Node inliningTarget, PSSLSocket socket,
243243
if (socket.hasSavedException()) {
244244
throw socket.getAndClearSavedException();
245245
}
246-
throw constructAndRaiseNode.get(inliningTarget).raiseOSError(frame, e.getErrorCode(), fromJavaStringNode.execute(e.getMessage(), TS_ENCODING), null, null);
246+
throw constructAndRaiseNode.get(inliningTarget).raiseOSError(frame, e.getErrorCode(), fromJavaStringNode.execute(e.getMessage(), TS_ENCODING));
247247
}
248248
break;
249249
case WANTS_WRITE:
@@ -263,7 +263,7 @@ static void doSocket(VirtualFrame frame, Node inliningTarget, PSSLSocket socket,
263263
if (socket.hasSavedException()) {
264264
throw socket.getAndClearSavedException();
265265
}
266-
throw constructAndRaiseNode.get(inliningTarget).raiseOSError(frame, e.getErrorCode(), fromJavaStringNode.execute(e.getMessage(), TS_ENCODING), null, null);
266+
throw constructAndRaiseNode.get(inliningTarget).raiseOSError(frame, e.getErrorCode(), fromJavaStringNode.execute(e.getMessage(), TS_ENCODING));
267267
}
268268
break;
269269
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/nodes/PConstructAndRaiseNode.java‎

Lines changed: 36 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -166,65 +166,80 @@ private static TruffleString getMessage(Exception exception) {
166166
return toTruffleStringUncached(exception.getMessage());
167167
}
168168

169-
private final PException raise(Frame frame, PythonBuiltinClassType err, TruffleString message, Object... formatArgs) {
169+
private PException raise(Frame frame, PythonBuiltinClassType err, TruffleString message, Object... formatArgs) {
170170
return executeWithFmtMessageAndArgs(frame, err, message, formatArgs, PythonUtils.EMPTY_OBJECT_ARRAY);
171171
}
172172

173+
private static Object[] createOsErrorArgs(int errno, TruffleString message) {
174+
return new Object[]{errno, message};
175+
}
176+
177+
private static Object[] createOsErrorArgs(int errno, TruffleString message, Object filename1) {
178+
return createOsErrorArgs(errno, message, filename1, null);
179+
}
180+
181+
private static Object[] createOsErrorArgs(OSErrorEnum osErrorEnum, TruffleString filename1) {
182+
return createOsErrorArgs(osErrorEnum.getNumber(), osErrorEnum.getMessage(), filename1, null);
183+
}
184+
173185
private static Object[] createOsErrorArgs(int errno, TruffleString message, Object filename1, Object filename2) {
174-
return new Object[]{errno, message, filename1, null, filename2};
186+
if (filename1 != null) {
187+
if (filename2 != null) {
188+
return new Object[]{errno, message, filename1, 0, filename2};
189+
}
190+
return new Object[]{errno, message, filename1};
191+
}
192+
assert filename2 == null;
193+
return new Object[]{errno, message};
175194
}
176195

177-
private static Object[] createOsErrorArgs(OSErrorEnum osErrorEnum, TruffleString filename1, TruffleString filename2) {
178-
return new Object[]{osErrorEnum.getNumber(), osErrorEnum.getMessage(), filename1, null, filename2};
196+
private static Object[] createOsErrorArgs(OSErrorEnum osErrorEnum) {
197+
return new Object[]{osErrorEnum.getNumber(), osErrorEnum.getMessage()};
179198
}
180199

181-
private static Object[] createOsErrorArgs(Exception exception, TruffleString filename1, TruffleString filename2, TruffleString.EqualNode eqNode) {
200+
private static Object[] createOsErrorArgs(Exception exception, TruffleString.EqualNode eqNode) {
182201
OSErrorEnum.ErrorAndMessagePair errorAndMessage = OSErrorEnum.fromException(exception, eqNode);
183-
return new Object[]{errorAndMessage.oserror.getNumber(), errorAndMessage.message, filename1, null, filename2};
202+
return new Object[]{errorAndMessage.oserror.getNumber(), errorAndMessage.message};
184203
}
185204

186205
private PException raiseOSErrorInternal(Frame frame, Object[] arguments) {
187206
return executeWithArgsOnly(frame, PythonBuiltinClassType.OSError, arguments);
188207
}
189208

190209
public final PException raiseOSError(Frame frame, OSErrorEnum osErrorEnum) {
191-
return raiseOSErrorInternal(frame, createOsErrorArgs(osErrorEnum, null, null));
210+
return raiseOSErrorInternal(frame, createOsErrorArgs(osErrorEnum));
192211
}
193212

194213
public final PException raiseOSError(Frame frame, OSErrorEnum osErrorEnum, TruffleString filename) {
195-
return raiseOSErrorInternal(frame, createOsErrorArgs(osErrorEnum, filename, null));
214+
return raiseOSErrorInternal(frame, createOsErrorArgs(osErrorEnum, filename));
196215
}
197216

198217
public final PException raiseOSError(Frame frame, Exception exception, TruffleString.EqualNode eqNode) {
199-
return raiseOSErrorInternal(frame, createOsErrorArgs(exception, null, null, eqNode));
218+
return raiseOSErrorInternal(frame, createOsErrorArgs(exception, eqNode));
200219
}
201220

202221
public final PException raiseOSError(Frame frame, OSErrorEnum osErrorEnum, Exception exception) {
203222
return raiseOSError(frame, osErrorEnum, getMessage(exception));
204223
}
205224

206225
public final PException raiseOSError(Frame frame, int errno, TruffleString message, Object filename) {
207-
return raiseOSErrorInternal(frame, createOsErrorArgs(errno, message, filename, null));
208-
}
209-
210-
public final PException raiseOSError(Frame frame, int errno, TruffleString message, Object filename, Object filename2) {
211-
return raiseOSErrorInternal(frame, createOsErrorArgs(errno, message, filename, filename2));
226+
return raiseOSErrorInternal(frame, createOsErrorArgs(errno, message, filename));
212227
}
213228

214-
public final PException raiseOSError(VirtualFrame frame, int code, TruffleString message) {
215-
return raiseOSError(frame, code, message, null, null);
229+
public final PException raiseOSError(VirtualFrame frame, int errno, TruffleString message) {
230+
return raiseOSErrorInternal(frame, createOsErrorArgs(errno, message));
216231
}
217232

218233
public final PException raiseOSErrorFromPosixException(VirtualFrame frame, PosixException e) {
219-
return raiseOSError(frame, e.getErrorCode(), e.getMessageAsTruffleString(), null, null);
234+
return raiseOSErrorInternal(frame, createOsErrorArgs(e.getErrorCode(), e.getMessageAsTruffleString()));
220235
}
221236

222237
public final PException raiseOSErrorFromPosixException(VirtualFrame frame, PosixException e, Object filename1) {
223-
return raiseOSError(frame, e.getErrorCode(), e.getMessageAsTruffleString(), filename1, null);
238+
return raiseOSErrorInternal(frame, createOsErrorArgs(e.getErrorCode(), e.getMessageAsTruffleString(), filename1));
224239
}
225240

226241
public final PException raiseOSErrorFromPosixException(VirtualFrame frame, PosixException e, Object filename1, Object filename2) {
227-
return raiseOSError(frame, e.getErrorCode(), e.getMessageAsTruffleString(), filename1, filename2);
242+
return raiseOSErrorInternal(frame, createOsErrorArgs(e.getErrorCode(), e.getMessageAsTruffleString(), filename1, filename2));
228243
}
229244

230245
public final PException raiseOSErrorUnsupported(VirtualFrame frame, UnsupportedPosixFeatureException e) {
@@ -240,7 +255,7 @@ public final PException raiseSSLError(Frame frame, TruffleString message) {
240255
return raiseSSLError(frame, message, PythonUtils.EMPTY_OBJECT_ARRAY);
241256
}
242257

243-
private final PException raiseSSLError(Frame frame, TruffleString message, Object... formatArgs) {
258+
private PException raiseSSLError(Frame frame, TruffleString message, Object... formatArgs) {
244259
return raise(frame, PythonBuiltinClassType.SSLError, message, formatArgs);
245260
}
246261

@@ -274,7 +289,7 @@ public static PException raiseUncachedSSLError(SSLErrorCode errorCode, TruffleSt
274289
return getUncached().raiseSSLError(null, errorCode, format, formatArgs);
275290
}
276291

277-
private final PException raiseOSErrorSubType(Frame frame, PythonBuiltinClassType osErrorSubtype, TruffleString format, Object... fmtArgs) {
292+
private PException raiseOSErrorSubType(Frame frame, PythonBuiltinClassType osErrorSubtype, TruffleString format, Object... fmtArgs) {
278293
TruffleString message = getFormattedMessage(format, fmtArgs);
279294
final OSErrorEnum osErrorEnum = errorType2errno(osErrorSubtype);
280295
assert osErrorEnum != null : "could not determine an errno for this error, either not an OSError subtype or multiple errno codes are available";
@@ -293,14 +308,6 @@ public final PException raiseUnicodeEncodeError(Frame frame, String encoding, Tr
293308
return executeWithArgsOnly(frame, PythonBuiltinClassType.UnicodeEncodeError, new Object[]{encoding, object, start, end, reason});
294309
}
295310

296-
private final PException raiseUnicodeDecodeError(Frame frame, String encoding, Object object, int start, int end, String reason) {
297-
return executeWithArgsOnly(frame, PythonBuiltinClassType.UnicodeDecodeError, new Object[]{encoding, object, start, end, reason});
298-
}
299-
300-
public static PException raiseUncachedUnicodeDecodeError(String encoding, Object object, int start, int end, String reason) {
301-
return getUncached().raiseUnicodeDecodeError(null, encoding, object, start, end, reason);
302-
}
303-
304311
@NeverDefault
305312
public static PConstructAndRaiseNode create() {
306313
return PConstructAndRaiseNodeGen.create();

0 commit comments

Comments
 (0)