Skip to content

Commit

Permalink
Fix PCRE2 error constants and gracefully handle disabled JIT
Browse files Browse the repository at this point in the history
When the JIT is disabled, the PCRE.jit_compile function would throw
to complain. That's not particularly nice, since we don't check before
whether the JIT is enabled. Instead, return a boolean to indicate whether
the jit was enabled (other errors still throw at jit compile time).
While I was at it, I noticed that we no longer have the ERROR_ flags
in pcre_h.jl (I'm sure we once did, because there's other parts of
this file that use them, in particular the error path in PCRE.info).
Adjust the regex to also capture those error falgs, which are specified
as e.g.:
```
```
Finally, add a test for the error path in PCRE.info to make sure it
throws an ErrorException, not an UndefVarError.
  • Loading branch information
Keno committed Jan 9, 2019
1 parent 6b108ec commit c9e5a6a
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 5 deletions.
4 changes: 2 additions & 2 deletions base/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@ endif

all: $(addprefix $(BUILDDIR)/,pcre_h.jl errno_h.jl build_h.jl.phony file_constants.jl uv_constants.jl version_git.jl.phony)

PCRE_CONST := 0x[0-9a-fA-F]+|[0-9]+
PCRE_CONST := 0x[0-9a-fA-F]+|[0-9]+|\([\-0-9]+\)
ifeq ($(USE_SYSTEM_PCRE), 1)
PCRE_INCL_PATH := $(shell $(PCRE_CONFIG) --prefix)/include/pcre2.h
else
PCRE_INCL_PATH := $(build_includedir)/pcre2.h
endif

$(BUILDDIR)/pcre_h.jl: $(PCRE_INCL_PATH)
@$(call PRINT_PERL, $(CPP) -D PCRE2_CODE_UNIT_WIDTH=8 -dM $< | perl -nle '/^\s*#define\s+PCRE2_(\w*)\s*\(?($(PCRE_CONST))\)?u?\s*$$/ and print "const $$1 = UInt32($$2)"' | LC_ALL=C sort > $@)
@$(call PRINT_PERL, $(CPP) -D PCRE2_CODE_UNIT_WIDTH=8 -dM $< | perl -nle '/^\s*#define\s+PCRE2_(\w*)\s*\(?($(PCRE_CONST))\)?u?\s*$$/ and print "const $$1 = $$2 % UInt32"' | LC_ALL=C sort > $@)

$(BUILDDIR)/errno_h.jl:
@$(call PRINT_PERL, echo '#include <errno.h>' | $(CPP) -dM - | perl -nle 'print "const $$1 = Int32($$2)" if /^#define\s+(E\w+)\s+(\d+)\s*$$/' | LC_ALL=C sort > $@)
Expand Down
8 changes: 5 additions & 3 deletions base/pcre.jl
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ function info(regex::Ptr{Cvoid}, what::Integer, ::Type{T}) where T
buf = RefValue{T}()
ret = ccall((:pcre2_pattern_info_8, PCRE_LIB), Int32,
(Ptr{Cvoid}, Int32, Ptr{Cvoid}),
regex, what, buf)
regex, what, buf) % UInt32
if ret != 0
error(ret == ERROR_NULL ? "NULL regex object" :
ret == ERROR_BADMAGIC ? "invalid regex object" :
Expand Down Expand Up @@ -106,8 +106,10 @@ end

function jit_compile(regex::Ptr{Cvoid})
errno = ccall((:pcre2_jit_compile_8, PCRE_LIB), Cint,
(Ptr{Cvoid}, UInt32), regex, JIT_COMPLETE)
errno == 0 || error("PCRE JIT error: $(err_message(errno))")
(Ptr{Cvoid}, UInt32), regex, JIT_COMPLETE) % UInt32
errno == 0 && return true
errno == ERROR_JIT_BADOPTION && return false
error("PCRE JIT error: $(err_message(errno))")
end

free_match_data(match_data) =
Expand Down
4 changes: 4 additions & 0 deletions test/regex.jl
Original file line number Diff line number Diff line change
Expand Up @@ -65,4 +65,8 @@

# Regex behaves like a scalar in broadcasting
@test occursin.(r"Hello", ["Hello", "World"]) == [true, false]

# Test that PCRE throws the correct kind of error
# TODO: Uncomment this once the corresponding change has propagated to CI
#@test_throws ErrorException Base.PCRE.info(C_NULL, Base.PCRE.INFO_NAMECOUNT, UInt32)
end

4 comments on commit c9e5a6a

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Executing the daily benchmark build, I will reply here when finished:

@nanosoldier runbenchmarks(ALL, isdaily = true)

@staticfloat
Copy link
Member

Choose a reason for hiding this comment

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

@Keno can you explain to me why you added the % UInt32 to these ccall's? They seem to be causing problems, because PCRE's error codes are negative. The error I get is InexactError(:check_top_bit, UInt32, 0xffffffd0), which is kind of confusing to me since I would think that that value should be representable as a UInt32.

@Keno
Copy link
Member Author

@Keno Keno commented on c9e5a6a Apr 2, 2019

Choose a reason for hiding this comment

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

Hmm? The point of the % UInt32 is to do the truncation without the conversion check:

julia> Int32(-1) % UInt32
0xffffffff

Are you sure you didn't accidentally have an old version of the Makefile that used the UInt32() form which would throw that error?

@staticfloat
Copy link
Member

Choose a reason for hiding this comment

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

I think you must be right, although I don't know why I would have had that old version. /shrug

Please # to comment.