Skip to content

Commit

Permalink
Prepend internal marker to internal sources
Browse files Browse the repository at this point in the history 
This allows tools to omit any of our internal Ruby-based kernel
sources when processing backtraces and source_locations. The
filtering here is only done in source_location and backtrace
generation currently, with some extra logic during backtrace
mining to skip internal frames for e.g. Kernel#warn.

I only did this filtering at the highest levels, when the filename
is actually about to be used and returned to a Ruby consumer, to
avoid any internal usage of these filenames breaking due to the
new prefix.

Fixes jruby#6430
  • Loading branch information
@headius
headius committed Oct 10, 2023
1 parent 96387a6 commit 641a091
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 30 deletions.
3 changes: 2 additions & 1 deletion core/src/main/java/org/jruby/AbstractRubyMethod.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import org.jruby.runtime.Helpers;
import org.jruby.runtime.PositionAware;
import org.jruby.runtime.ThreadContext;
import org.jruby.runtime.backtrace.TraceType;
import org.jruby.runtime.builtin.IRubyObject;
import org.jruby.runtime.callsite.CacheEntry;
import org.jruby.runtime.marshal.DataType;
Expand Down Expand Up @@ -138,7 +139,7 @@ public String getFilename() {
DynamicMethod realMethod = method.getRealMethod(); // Follow Aliases
if (realMethod instanceof PositionAware) {
PositionAware poser = (PositionAware) realMethod;
return poser.getFile();
return TraceType.maskInternalFiles(poser.getFile());
}
return null;
}
Expand Down
19 changes: 1 addition & 18 deletions core/src/main/java/org/jruby/RubyMethod.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
import org.jruby.runtime.PositionAware;
import org.jruby.runtime.Signature;
import org.jruby.runtime.ThreadContext;
import org.jruby.runtime.backtrace.TraceType;
import org.jruby.runtime.builtin.IRubyObject;
import org.jruby.runtime.callsite.CacheEntry;

Expand Down Expand Up @@ -358,24 +359,6 @@ public IRubyObject source_location(ThreadContext context) {
return context.nil;
}

public String getFilename() {
DynamicMethod realMethod = method.getRealMethod(); // Follow Aliases
if (realMethod instanceof PositionAware) {
PositionAware poser = (PositionAware) realMethod;
return poser.getFile();
}
return null;
}

public int getLine() {
DynamicMethod realMethod = method.getRealMethod(); // Follow Aliases
if (realMethod instanceof PositionAware) {
PositionAware poser = (PositionAware) realMethod;
return poser.getLine() + 1;
}
return -1;
}

@JRubyMethod
public IRubyObject parameters(ThreadContext context) {
return Helpers.methodToParameters(context.runtime, this);
Expand Down
15 changes: 7 additions & 8 deletions core/src/main/java/org/jruby/runtime/ThreadContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -910,16 +910,15 @@ public IRubyObject createCallerLocations(int level, Integer length, Stream<Stack
return backTrace;
}

private RubyStackTraceElement[] getFullTrace(Integer length, Stream<StackWalker.StackFrame> stackStream) {
if (length != null && length == 0) return RubyStackTraceElement.EMPTY_ARRAY;
return TraceType.Gather.CALLER.getBacktraceData(this, stackStream).getBacktrace(runtime);
}

private RubyStackTraceElement[] getPartialTrace(int level, Integer length, Stream<StackWalker.StackFrame> stackStream) {
if (length != null && length == 0) return RubyStackTraceElement.EMPTY_ARRAY;
return TraceType.Gather.CALLER.getBacktraceData(this, stackStream).getPartialBacktrace(runtime, level + length);
}

private RubyStackTraceElement[] getWarnTrace(int level, Stream<StackWalker.StackFrame> stackStream) {
return TraceType.Gather.WARN.getBacktraceData(this, stackStream).getPartialBacktrace(runtime, level + 1);
}

private static int safeLength(int level, Integer length, RubyStackTraceElement[] trace) {
final int baseLength = trace.length - level;
return Math.min(length, baseLength);
Expand All @@ -935,7 +934,7 @@ private static int safeLength(int level, Integer length, RubyStackTraceElement[]
public RubyStackTraceElement getSingleBacktrace(int level) {
runtime.incrementWarningCount();

RubyStackTraceElement[] trace = WALKER.walk(stream -> getPartialTrace(level, 1, stream));
RubyStackTraceElement[] trace = WALKER.walk(stream -> getWarnTrace(level, stream));

if (RubyInstanceConfig.LOG_WARNINGS) TraceType.logWarning(trace);

Expand All @@ -948,7 +947,7 @@ public RubyStackTraceElement getSingleBacktrace(int level) {
public RubyStackTraceElement getSingleBacktraceExact(int level) {
runtime.incrementWarningCount();

RubyStackTraceElement[] trace = WALKER.walk(stream -> getPartialTrace(level, 1, stream));
RubyStackTraceElement[] trace = WALKER.walk(stream -> getWarnTrace(level, stream));

if (RubyInstanceConfig.LOG_WARNINGS) TraceType.logWarning(trace);

Expand Down Expand Up @@ -991,7 +990,7 @@ public final Stream<BacktraceElement> getBacktrace(int level) {
public static String createRawBacktraceStringFromThrowable(final Throwable ex, final boolean color) {
return WALKER.walk(ex.getStackTrace(), stream ->
TraceType.printBacktraceJRuby(null,
new BacktraceData(stream, Stream.empty(), true, true, false, false).getBacktraceWithoutRuby(),
new BacktraceData(stream, Stream.empty(), true, true, false, false, false).getBacktraceWithoutRuby(),
ex.getClass().getName(),
ex.getLocalizedMessage(),
color));
Expand Down
21 changes: 19 additions & 2 deletions core/src/main/java/org/jruby/runtime/backtrace/BacktraceData.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,16 @@ public class BacktraceData implements Serializable {
private final boolean rawTrace;
private final boolean maskNative;
private final boolean includeNonFiltered;
private final boolean excludeInternal;

public BacktraceData(Stream<StackWalker.StackFrame> stackStream, Stream<BacktraceElement> rubyTrace, boolean fullTrace, boolean rawTrace, boolean maskNative, boolean includeNonFiltered) {
public BacktraceData(Stream<StackWalker.StackFrame> stackStream, Stream<BacktraceElement> rubyTrace, boolean fullTrace, boolean rawTrace, boolean maskNative, boolean includeNonFiltered, boolean excludeInternal) {
this.stackStream = stackStream;
this.rubyTrace = rubyTrace;
this.fullTrace = fullTrace;
this.rawTrace = rawTrace;
this.maskNative = maskNative;
this.includeNonFiltered = includeNonFiltered;
this.excludeInternal = excludeInternal;
}

public static final BacktraceData EMPTY = new BacktraceData(
Expand All @@ -40,6 +42,7 @@ public BacktraceData(Stream<StackWalker.StackFrame> stackStream, Stream<Backtrac
false,
false,
false,
false,
false);

public final RubyStackTraceElement[] getBacktrace(Ruby runtime) {
Expand Down Expand Up @@ -89,6 +92,9 @@ private RubyStackTraceElement[] constructBacktrace(Map<String, Map<String, Strin

// Only rewrite non-Java files when not in "raw" mode
if (!(rawTrace || filename == null || filename.endsWith(".java"))) {
// skip internal sources if requested
if (excludeInternal && TraceType.isInternal(filename)) continue;

List<String> mangledTuple = JavaNameMangler.decodeMethodTuple(methodName);
if (mangledTuple != null) {
FrameType type = JavaNameMangler.decodeFrameTypeFromMangledName(mangledTuple.get(1));
Expand All @@ -103,6 +109,9 @@ private RubyStackTraceElement[] constructBacktrace(Map<String, Map<String, Strin
continue;
}

// mask internal file paths
filename = TraceType.maskInternalFiles(filename);

// construct Ruby trace element
RubyStackTraceElement rubyElement = new RubyStackTraceElement(className, decodedName, filename, line, false, type);

Expand Down Expand Up @@ -161,7 +170,15 @@ private RubyStackTraceElement[] constructBacktrace(Map<String, Map<String, Strin
break;
default: newName = rubyFrame.method;
}
RubyStackTraceElement rubyElement = new RubyStackTraceElement("RUBY", newName, rubyFrame.filename, rubyFrame.line + 1, false, frameType);

// skip internal sources if requested
filename = rubyFrame.filename;
if (excludeInternal && TraceType.isInternal(filename)) continue;

// mask internal file paths
filename = TraceType.maskInternalFiles(filename);

RubyStackTraceElement rubyElement = new RubyStackTraceElement("RUBY", newName, filename, rubyFrame.line + 1, false, frameType);

// dup if masking native and previous frame was native
if (maskNative && dupFrame) {
Expand Down
36 changes: 35 additions & 1 deletion core/src/main/java/org/jruby/runtime/backtrace/TraceType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ public BacktraceData getBacktraceData(ThreadContext context, Stream<StackWalker.
true,
true,
false,
false,
false);
}
},
Expand All @@ -187,6 +188,7 @@ public BacktraceData getBacktraceData(ThreadContext context, Stream<StackWalker.
true,
false,
false,
false,
false);
}
},
Expand All @@ -202,7 +204,8 @@ public BacktraceData getBacktraceData(ThreadContext context, Stream<StackWalker.
false,
false,
false,
true);
true,
false);
}
},

Expand All @@ -217,6 +220,7 @@ public BacktraceData getBacktraceData(ThreadContext context, Stream<StackWalker.
false,
false,
context.runtime.getInstanceConfig().getBacktraceMask(),
false,
false);
}
},
Expand All @@ -232,8 +236,25 @@ public BacktraceData getBacktraceData(ThreadContext context, Stream<StackWalker.
false,
false,
true,
false,
false);
}
},

/**
* Warning traces, showing only Ruby and core class methods, excluding internal files.
*/
WARN {
public BacktraceData getBacktraceData(ThreadContext context, Stream<StackWalker.StackFrame> stackStream) {
return new BacktraceData(
stackStream,
context.getBacktrace(),
false,
false,
true,
false,
true);
}
};

/**
Expand Down Expand Up @@ -581,6 +602,19 @@ public static IRubyObject generateMRIBacktrace(Ruby runtime, RubyStackTraceEleme
return RubyArray.newArrayMayCopy(runtime, traceArray);
}

public static String maskInternalFiles(String filename) {
if (isInternal(filename)) {
return "<internal:" + filename + ">";
}

return filename;
}

public static boolean isInternal(String filename) {
return filename.startsWith("uri:classloader:/jruby/kernel/") ||
filename.startsWith("<internal:");
}

@Deprecated
public RubyStackTraceElement getBacktraceElement(ThreadContext context, int uplevel) {
// NOTE: could be optimized not to walk the whole stack
Expand Down

0 comments on commit 641a091

Please sign in to comment.