On Mon, 2009-09-07 at 22:41 +0200, Tomek Grabiec wrote:
> This fixes incorrect lock order reported by helgrind:
OK, so what is the problem? How are you fixing it? The changelog is
really too terse for this kind of critical fix.
> Signed-off-by: Tomek Grabiec <tgrab...@gmail.com>
> ---
> arch/x86/emit-code.c | 12 +++---------
> arch/x86/insn-selector.brg | 6 +++---
> include/jit/compilation-unit.h | 1 +
> include/jit/compiler.h | 11 +++++++++--
> jit/compilation-unit.c | 18 ++++++++++++++++++
> jit/emit.c | 12 ++++++++++++
> jit/fixup-site.c | 27 +++++++++++++++++++++++----
> test/jit/Makefile | 1 +
> 8 files changed, 70 insertions(+), 18 deletions(-)
>
> diff --git a/arch/x86/emit-code.c b/arch/x86/emit-code.c
> index ea96a03..f7b2e2a 100644
> --- a/arch/x86/emit-code.c
> +++ b/arch/x86/emit-code.c
> @@ -338,11 +338,9 @@ void fixup_direct_calls(struct jit_trampoline *t,
> unsigned long target)
>
> pthread_mutex_lock(&t->mutex);
>
> - list_for_each_entry_safe(this, next, &t->fixup_site_list,
> - fixup_list_node) {
> + list_for_each_entry_safe(this, next, &t->fixup_site_list,
> trampoline_node) {
> unsigned char *site_addr;
> uint32_t new_target;
> - bool is_compiled;
>
> /*
> * It is possible that we're fixing calls to
> @@ -352,18 +350,14 @@ void fixup_direct_calls(struct jit_trampoline *t,
> unsigned long target)
> * be set yet. We should skip fixing callsites coming
> * from not yet compiled methods. .
> */
> - pthread_mutex_lock(&this->cu->mutex);
> - is_compiled = this->cu->is_compiled;
> - pthread_mutex_unlock(&this->cu->mutex);
> -
> - if (!is_compiled)
> + if (!fixup_site_is_ready(this))
> continue;
>
> site_addr = fixup_site_addr(this);
> new_target = target - ((unsigned long) site_addr +
> CALL_INSN_SIZE);
> cpu_write_u32(site_addr+1, new_target);
>
> - list_del(&this->fixup_list_node);
> + list_del(&this->trampoline_node);
> free_fixup_site(this);
> }
>
> diff --git a/arch/x86/insn-selector.brg b/arch/x86/insn-selector.brg
> index 8eead10..5383368 100644
> --- a/arch/x86/insn-selector.brg
> +++ b/arch/x86/insn-selector.brg
> @@ -3207,9 +3207,9 @@ static void invoke(struct basic_block *s, struct
> tree_node *tree)
> if (!is_compiled) {
> struct fixup_site *fixup;
>
> - fixup = alloc_fixup_site();
> - fixup->cu = s->b_parent;
> - fixup->relcall_insn = call_insn;
> + fixup = alloc_fixup_site(s->b_parent, call_insn);
> + if (!fixup)
> + error("out of memory");
>
> trampoline_add_fixup_site(method->trampoline, fixup);
> }
> diff --git a/include/jit/compilation-unit.h b/include/jit/compilation-unit.h
> index c3f55b1..4bfd9d8 100644
> --- a/include/jit/compilation-unit.h
> +++ b/include/jit/compilation-unit.h
> @@ -62,6 +62,7 @@ struct compilation_unit {
> unsigned char *unwind_bb_ptr;
>
> struct list_head static_fixup_site_list;
> + struct list_head call_fixup_site_list;
> struct list_head tableswitch_list;
> struct list_head lookupswitch_list;
>
> diff --git a/include/jit/compiler.h b/include/jit/compiler.h
> index 6e1196a..7e5796b 100644
> --- a/include/jit/compiler.h
> +++ b/include/jit/compiler.h
> @@ -19,12 +19,17 @@ struct statement;
> struct buffer;
>
> struct fixup_site {
> + pthread_mutex_t mutex;
> + bool ready;
> +
> + struct jit_trampoline *target;
> /* Compilation unit to which relcall_insn belongs */
> struct compilation_unit *cu;
> /* We need this, because we don't have native pointer at
> instruction selection */
> struct insn *relcall_insn;
> - struct list_head fixup_list_node;
> + struct list_head trampoline_node;
> + struct list_head cu_node;
> };
>
> struct jit_trampoline {
> @@ -65,10 +70,12 @@ void *jit_magic_trampoline(struct compilation_unit *);
> struct jit_trampoline *alloc_jit_trampoline(void);
> struct jit_trampoline *build_jit_trampoline(struct compilation_unit *);
> void free_jit_trampoline(struct jit_trampoline *);
> -struct fixup_site *alloc_fixup_site(void);
> +struct fixup_site *alloc_fixup_site(struct compilation_unit *, struct insn
> *);
> void free_fixup_site(struct fixup_site *);
> void trampoline_add_fixup_site(struct jit_trampoline *, struct fixup_site *);
> unsigned char *fixup_site_addr(struct fixup_site *);
> +bool fixup_site_is_ready(struct fixup_site *);
> +void prepare_call_fixup_sites(struct compilation_unit *);
>
> const char *method_symbol(struct vm_method *method, char *symbol, size_t
> len);
>
> diff --git a/jit/compilation-unit.c b/jit/compilation-unit.c
> index 24c5738..e0c4be3 100644
> --- a/jit/compilation-unit.c
> +++ b/jit/compilation-unit.c
> @@ -95,6 +95,7 @@ struct compilation_unit *compilation_unit_alloc(struct
> vm_method *method)
> goto out_of_memory;
>
> INIT_LIST_HEAD(&cu->static_fixup_site_list);
> + INIT_LIST_HEAD(&cu->call_fixup_site_list);
> INIT_LIST_HEAD(&cu->tableswitch_list);
> INIT_LIST_HEAD(&cu->lookupswitch_list);
>
> @@ -162,6 +163,22 @@ static void free_lookupswitch_list(struct
> compilation_unit *cu)
> }
> }
>
> +static void free_call_fixup_sites(struct compilation_unit *cu)
> +{
> + struct fixup_site *this, *next;
> +
> + list_for_each_entry_safe(this, next, &cu->call_fixup_site_list, cu_node)
> + {
> + list_del(&this->cu_node);
> +
> + pthread_mutex_lock(&this->target->mutex);
> + list_del(&this->trampoline_node);
> + pthread_mutex_unlock(&this->target->mutex);
> +
> + free_fixup_site(this);
> + }
> +}
> +
> static void free_lir_insn_map(struct compilation_unit *cu)
> {
> free_radix_tree(cu->lir_insn_map);
> @@ -178,6 +195,7 @@ void free_compilation_unit(struct compilation_unit *cu)
> {
> struct basic_block *bb, *tmp_bb;
>
> + free_call_fixup_sites(cu);
> shrink_compilation_unit(cu);
>
> list_for_each_entry_safe(bb, tmp_bb, &cu->bb_list, bb_list_node)
> diff --git a/jit/emit.c b/jit/emit.c
> index ee5c4d9..de0a1ec 100644
> --- a/jit/emit.c
> +++ b/jit/emit.c
> @@ -152,6 +152,17 @@ static void emit_resolution_blocks(struct basic_block
> *bb, struct buffer *buf)
> }
> }
>
> +void prepare_call_fixup_sites(struct compilation_unit *cu)
> +{
> + struct fixup_site *site;
> +
> + list_for_each_entry(site, &cu->call_fixup_site_list, cu_node) {
> + pthread_mutex_lock(&site->mutex);
> + site->ready = true;
> + pthread_mutex_unlock(&site->mutex);
> + }
> +}
> +
> int emit_machine_code(struct compilation_unit *cu)
> {
> unsigned long frame_size;
> @@ -196,6 +207,7 @@ int emit_machine_code(struct compilation_unit *cu)
> emit_resolution_blocks(bb, cu->objcode);
> }
>
> + prepare_call_fixup_sites(cu);
> backpatch_tableswitch_targets(cu);
> backpatch_lookupswitch_targets(cu);
> build_exception_handlers_table(cu);
> diff --git a/jit/fixup-site.c b/jit/fixup-site.c
> index 0d38d8a..ee48681 100644
> --- a/jit/fixup-site.c
> +++ b/jit/fixup-site.c
> @@ -26,24 +26,32 @@
>
> #include "jit/compiler.h"
> #include "arch/instruction.h"
> +#include "vm/stdlib.h"
> +
> #include <memory.h>
> #include <malloc.h>
>
> -struct fixup_site *alloc_fixup_site(void)
> +struct fixup_site *
> +alloc_fixup_site(struct compilation_unit *cu, struct insn *call_insn)
> {
> struct fixup_site *site;
>
> - site = malloc(sizeof(*site));
> + site = zalloc(sizeof(*site));
> if (!site)
> return NULL;
>
> - memset(site, 0, sizeof(*site));
> + site->cu = cu;
> + site->relcall_insn = call_insn;
> +
> + list_add(&site->cu_node, &cu->call_fixup_site_list);
> + pthread_mutex_init(&site->mutex, NULL);
>
> return site;
> }
>
> void free_fixup_site(struct fixup_site *site)
> {
> + pthread_mutex_destroy(&site->mutex);
> free(site);
> }
>
> @@ -51,7 +59,7 @@ void trampoline_add_fixup_site(struct jit_trampoline
> *trampoline,
> struct fixup_site *site)
> {
> pthread_mutex_lock(&trampoline->mutex);
> - list_add_tail(&site->fixup_list_node, &trampoline->fixup_site_list);
> + list_add_tail(&site->trampoline_node, &trampoline->fixup_site_list);
> pthread_mutex_unlock(&trampoline->mutex);
> }
>
> @@ -59,3 +67,14 @@ unsigned char *fixup_site_addr(struct fixup_site *site)
> {
> return buffer_ptr(site->cu->objcode) + site->relcall_insn->mach_offset;
> }
> +
> +bool fixup_site_is_ready(struct fixup_site *site)
> +{
> + bool result;
> +
> + pthread_mutex_lock(&site->mutex);
> + result = site->ready;
> + pthread_mutex_unlock(&site->mutex);
> +
> + return result;
> +}
> diff --git a/test/jit/Makefile b/test/jit/Makefile
> index 3bb8aa1..17c8a4c 100644
> --- a/test/jit/Makefile
> +++ b/test/jit/Makefile
> @@ -24,6 +24,7 @@ TOPLEVEL_OBJS := \
> jit/exception-bc.o \
> jit/exception.o \
> jit/expression.o \
> + jit/fixup-site.o \
> jit/interval.o \
> jit/invoke-bc.o \
> jit/linear-scan.o \
------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what's new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july
_______________________________________________
Jatovm-devel mailing list
Jatovm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/jatovm-devel
