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

RFC: implement proper debug mode for packages (with support for re-precompilation) #37874

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,11 @@ New library functions
efficiently ([#35816]).
* New function `addenv` for adding environment mappings into a `Cmd` object, returning the new `Cmd` object.
* New function `insorted` for determining whether an element is in a sorted collection or not ([#37490]).
* New function `isdebug` for checking if julia is running in debug mode.
Packages use different precompile caches in debug and non-debug mode thus it
safe to use this function for compile time decisions. Also, note that
`@assert` now is a no-op unless `isdebug()` is true. Debug mode is activated
with the `-g2` flag.

New library features
--------------------
Expand Down Expand Up @@ -124,6 +129,8 @@ Standard library changes
* The `Pkg.Artifacts` module has been imported as a separate standard library. It is still available as
`Pkg.Artifacts`, however starting from Julia v1.6+, packages may import simply `Artifacts` without importing
all of `Pkg` alongside. ([#37320])
* The `@assert` macro is now only active when julia is started with a debug
level of 2 (`julia -g2`).

#### LinearAlgebra

Expand Down
4 changes: 4 additions & 0 deletions base/Base.jl
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ time_ns() = ccall(:jl_hrtime, UInt64, ())

start_base_include = time_ns()

julia_debug = true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not make this a Ref?

isdebug() = julia_debug::Bool

## Load essential files and libraries
include("essentials.jl")
include("ctypes.jl")
Expand Down Expand Up @@ -396,6 +399,7 @@ in_sysimage(pkgid::PkgId) = pkgid in _sysimage_modules

if is_primary_base_module
function __init__()
global julia_debug = Base.JLOptions().debug_level >= 2
# try to ensuremake sure OpenBLAS does not set CPU affinity (#1070, #9639)
if !haskey(ENV, "OPENBLAS_MAIN_FREE") && !haskey(ENV, "GOTOBLAS_MAIN_FREE")
ENV["OPENBLAS_MAIN_FREE"] = "1"
Expand Down
23 changes: 14 additions & 9 deletions base/error.jl
Original file line number Diff line number Diff line change
Expand Up @@ -183,21 +183,25 @@ windowserror(p, b::Bool; extrainfo=nothing) = b ? windowserror(p, extrainfo=extr
windowserror(p, code::UInt32=Libc.GetLastError(); extrainfo=nothing) = throw(Main.Base.SystemError(string(p), 0, WindowsErrorInfo(code, extrainfo)))


## assertion macro ##
## assertion and ifdebug macros ##

"""
@ifdebug expr

Equivalent to `@static if isdebug()`
"""
macro ifdebug(expr)
isdefined(@__MODULE__, :isdebug) && !(isdebug()) && return nothing
return :(esc(expr))
end


"""
@assert cond [text]

Throw an [`AssertionError`](@ref) if `cond` is `false`. Preferred syntax for writing assertions.
Message `text` is optionally displayed upon assertion failure.

!!! warning
An assert might be disabled at various optimization levels.
Assert should therefore only be used as a debugging tool
and not used for authentication verification (e.g., verifying passwords),
nor should side effects needed for the function to work correctly
be used inside of asserts.
Message `text` is optionally displayed upon assertion failure. Only active when julia is started
in debug mode (`julia -g`).

# Examples
```jldoctest
Expand All @@ -208,6 +212,7 @@ julia> @assert isodd(3) "What even are numbers?"
```
"""
macro assert(ex, msgs...)
isdefined(@__MODULE__, :isdebug) && !(isdebug()) && return nothing
msg = isempty(msgs) ? ex : msgs[1]
if isa(msg, AbstractString)
msg = msg # pass-through
Expand Down
1 change: 1 addition & 0 deletions base/exports.jl
Original file line number Diff line number Diff line change
Expand Up @@ -780,6 +780,7 @@ export
atreplinit,
exit,
ntuple,
isdebug,

# I/O and events
close,
Expand Down
20 changes: 14 additions & 6 deletions base/loading.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1231,6 +1231,7 @@ function compilecache_path(pkg::PkgId, cache::TOMLCache)::String
crc = _crc32c(unsafe_string(JLOptions().image_file), crc)
crc = _crc32c(unsafe_string(JLOptions().julia_bin), crc)
crc = _crc32c(get_preferences_hash(pkg.uuid, cache), crc)
crc = _crc32c(isdebug(), crc)
project_precompile_slug = slug(crc, 5)
abspath(cachepath, string(entryfile, "_", project_precompile_slug, ".ji"))
end
Expand Down Expand Up @@ -1358,6 +1359,8 @@ function parse_cache_header(f::IO)
end
totbytes -= 4 + 4 + n2 + 8
end
julia_debug = Bool(read(f, UInt8))
totbytes -= 1
prefs_hash = read(f, UInt64)
totbytes -= 8
@assert totbytes == 12 "header of cache file appears to be corrupt"
Expand All @@ -1372,7 +1375,7 @@ function parse_cache_header(f::IO)
build_id = read(f, UInt64) # build id
push!(required_modules, PkgId(uuid, sym) => build_id)
end
return modules, (includes, requires), required_modules, srctextpos, prefs_hash
return modules, (includes, requires), required_modules, srctextpos, prefs_hash, julia_debug
end

function parse_cache_header(cachefile::String; srcfiles_only::Bool=false)
Expand All @@ -1381,21 +1384,21 @@ function parse_cache_header(cachefile::String; srcfiles_only::Bool=false)
!isvalid_cache_header(io) && throw(ArgumentError("Invalid header in cache file $cachefile."))
ret = parse_cache_header(io)
srcfiles_only || return ret
modules, (includes, requires), required_modules, srctextpos, prefs_hash = ret
modules, (includes, requires), required_modules, srctextpos, prefs_hash, julia_debug = ret
srcfiles = srctext_files(io, srctextpos)
delidx = Int[]
for (i, chi) in enumerate(includes)
chi.filename ∈ srcfiles || push!(delidx, i)
end
deleteat!(includes, delidx)
return modules, (includes, requires), required_modules, srctextpos, prefs_hash
return modules, (includes, requires), required_modules, srctextpos, prefs_hash, julia_debug
finally
close(io)
end
end

function cache_dependencies(f::IO)
defs, (includes, requires), modules, srctextpos, prefs_hash = parse_cache_header(f)
defs, (includes, requires), modules, srctextpos, prefs_hash, julia_debug = parse_cache_header(f)
return modules, map(chi -> (chi.filename, chi.mtime), includes) # return just filename and mtime
end

Expand All @@ -1410,7 +1413,7 @@ function cache_dependencies(cachefile::String)
end

function read_dependency_src(io::IO, filename::AbstractString)
modules, (includes, requires), required_modules, srctextpos, prefs_hash = parse_cache_header(io)
modules, (includes, requires), required_modules, srctextpos, prefs_hash, julia_debug = parse_cache_header(io)
srctextpos == 0 && error("no source-text stored in cache file")
seek(io, srctextpos)
return _read_dependency_src(io, filename)
Expand Down Expand Up @@ -1496,7 +1499,7 @@ function stale_cachefile(modpath::String, cachefile::String, cache::TOMLCache)
@debug "Rejecting cache file $cachefile due to it containing an invalid cache header"
return true # invalid cache file
end
modules, (includes, requires), required_modules, srctextpos, prefs_hash = parse_cache_header(io)
modules, (includes, requires), required_modules, srctextpos, prefs_hash, julia_debug = parse_cache_header(io)
id = isempty(modules) ? nothing : first(modules).first
modules = Dict{PkgId, UInt64}(modules)

Expand Down Expand Up @@ -1572,6 +1575,11 @@ function stale_cachefile(modpath::String, cachefile::String, cache::TOMLCache)
end

if isa(id, PkgId)
curr_debug = isdebug()
if julia_debug != curr_debug
@debug "Rejecting cache file $cachefile because julia debug mode does not match"
return true
end
curr_prefs_hash = get_preferences_hash(id.uuid, cache)
if prefs_hash != curr_prefs_hash
@debug "Rejecting cache file $cachefile because preferences hash does not match 0x$(string(prefs_hash, base=16)) != 0x$(string(curr_prefs_hash, base=16))"
Expand Down
2 changes: 1 addition & 1 deletion doc/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ SRCDIR := $(abspath $(dir $(lastword $(MAKEFILE_LIST))))
JULIAHOME := $(abspath $(SRCDIR)/..)
SRCCACHE := $(abspath $(JULIAHOME)/deps/srccache)
include $(JULIAHOME)/Make.inc
JULIA_EXECUTABLE := $(call spawn,$(build_bindir)/julia) --startup-file=no
JULIA_EXECUTABLE := $(call spawn,$(build_bindir)/julia) --startup-file=no -g2

.PHONY: help clean cleanall html pdf deps deploy

Expand Down
8 changes: 8 additions & 0 deletions src/dump.c
Original file line number Diff line number Diff line change
Expand Up @@ -1124,6 +1124,14 @@ static int64_t write_dependency_list(ios_t *s, jl_array_t **udepsp, jl_array_t *
}
write_int32(s, 0); // terminator, for ease of reading

// Julia debug mode
jl_value_t *julia_debug = (jl_value_t*)jl_get_global(jl_base_module, jl_symbol("julia_debug"));
if (julia_debug) {
Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps this should call isdebug instead of going for the global?

write_int8(s, jl_unbox_bool(julia_debug));
} else {
write_int8(s, 0);
}

// Calculate Preferences hash for current package.
jl_value_t *prefs_hash = NULL;
if (jl_base_module) {
Expand Down
60 changes: 60 additions & 0 deletions test/assert.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
# This file is run from test/misc.jl separately since it needs to run with `-g2`.
using Test

# test @assert macro
@test_throws AssertionError (@assert 1 == 2)
@test_throws AssertionError (@assert false)
@test_throws AssertionError (@assert false "this is a test")
@test_throws AssertionError (@assert false "this is a test" "another test")
@test_throws AssertionError (@assert false :a)
let
try
@assert 1 == 2
error("unexpected")
catch ex
@test isa(ex, AssertionError)
@test occursin("1 == 2", ex.msg)
end
end
# test @assert message
let
try
@assert 1 == 2 "this is a test"
error("unexpected")
catch ex
@test isa(ex, AssertionError)
@test ex.msg == "this is a test"
end
end
# @assert only uses the first message string
let
try
@assert 1 == 2 "this is a test" "this is another test"
error("unexpected")
catch ex
@test isa(ex, AssertionError)
@test ex.msg == "this is a test"
end
end
# @assert calls string() on second argument
let
try
@assert 1 == 2 :random_object
error("unexpected")
catch ex
@test isa(ex, AssertionError)
@test !occursin("1 == 2", ex.msg)
@test occursin("random_object", ex.msg)
end
end
# if the second argument is an expression, c
let deepthought(x, y) = 42
try
@assert 1 == 2 string("the answer to the ultimate question: ",
deepthought(6, 9))
error("unexpected")
catch ex
@test isa(ex, AssertionError)
@test ex.msg == "the answer to the ultimate question: 42"
end
end
2 changes: 1 addition & 1 deletion test/errorshow.jl
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ let undefvar
@test err_str == "InterruptException:"
err_str = @except_str throw(ArgumentError("not an error")) ArgumentError
@test err_str == "ArgumentError: not an error"
err_str = @except_str @assert(false) AssertionError
err_str = read(`$(Base.julia_cmd()) -g2 -e 'try @assert false; catch e; showerror(stdout, e); end'`, String)
@test err_str == "AssertionError: false"
end

Expand Down
4 changes: 4 additions & 0 deletions test/exceptions.jl
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,8 @@ end
end

# issue #36527
s = raw"""
using Test
function f36527()
caught = false
🏡 = Core.eval(Main, :(module asdf36527 end))
Expand All @@ -383,6 +385,8 @@ function f36527()
end

@test f36527()
"""
@test success(`$(Base.julia_cmd()) -g2 -e $s`)

# accessing an undefined var in tail position in a catch block
function undef_var_in_catch()
Expand Down
70 changes: 7 additions & 63 deletions test/misc.jl
Original file line number Diff line number Diff line change
Expand Up @@ -3,64 +3,8 @@
isdefined(Main, :FakePTYs) || @eval Main include("testhelpers/FakePTYs.jl")

# Tests that do not really go anywhere else

# test @assert macro
@test_throws AssertionError (@assert 1 == 2)
@test_throws AssertionError (@assert false)
@test_throws AssertionError (@assert false "this is a test")
@test_throws AssertionError (@assert false "this is a test" "another test")
@test_throws AssertionError (@assert false :a)
let
try
@assert 1 == 2
error("unexpected")
catch ex
@test isa(ex, AssertionError)
@test occursin("1 == 2", ex.msg)
end
end
# test @assert message
let
try
@assert 1 == 2 "this is a test"
error("unexpected")
catch ex
@test isa(ex, AssertionError)
@test ex.msg == "this is a test"
end
end
# @assert only uses the first message string
let
try
@assert 1 == 2 "this is a test" "this is another test"
error("unexpected")
catch ex
@test isa(ex, AssertionError)
@test ex.msg == "this is a test"
end
end
# @assert calls string() on second argument
let
try
@assert 1 == 2 :random_object
error("unexpected")
catch ex
@test isa(ex, AssertionError)
@test !occursin("1 == 2", ex.msg)
@test occursin("random_object", ex.msg)
end
end
# if the second argument is an expression, c
let deepthought(x, y) = 42
try
@assert 1 == 2 string("the answer to the ultimate question: ",
deepthought(6, 9))
error("unexpected")
catch ex
@test isa(ex, AssertionError)
@test ex.msg == "the answer to the ultimate question: 42"
end
end
assert_file = joinpath(@__DIR__, "assert.jl")
@test success(`$(Base.julia_cmd()) -g2 $assert_file`)

let # test the process title functions, issue #9957
oldtitle = Sys.get_process_title()
Expand Down Expand Up @@ -770,19 +714,19 @@ end

@kwdef struct TestInnerConstructor
a = 1
TestInnerConstructor(a::Int) = (@assert a>0; new(a))
TestInnerConstructor(a::Int) = (a>0 || error(); new(a))
function TestInnerConstructor(a::String)
@assert length(a) > 0
length(a) > 0 || error()
new(a)
end
end

@testset "@kwdef inner constructor" begin
@test TestInnerConstructor() == TestInnerConstructor(1)
@test TestInnerConstructor(a=2) == TestInnerConstructor(2)
@test_throws AssertionError TestInnerConstructor(a=0)
@test_throws ErrorException TestInnerConstructor(a=0)
@test TestInnerConstructor(a="2") == TestInnerConstructor("2")
@test_throws AssertionError TestInnerConstructor(a="")
@test_throws ErrorException TestInnerConstructor(a="")
end

const outsidevar = 7
Expand Down Expand Up @@ -819,7 +763,7 @@ end

@testset "Pointer to unsigned/signed integer" begin
# assuming UInt and Ptr have the same size
@assert sizeof(UInt) == sizeof(Ptr{Nothing})
@test sizeof(UInt) == sizeof(Ptr{Nothing})
uint = UInt(0x12345678)
sint = signed(uint)
ptr = reinterpret(Ptr{Nothing}, uint)
Expand Down
Loading