Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/exec/exec-all.h | 10 ++++++++++
 bsd-user/mmap.c         |  1 +
 linux-user/mmap.c       |  1 +
 3 files changed, 12 insertions(+)

diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 5fa0687cd2..d02517e95f 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -629,6 +629,15 @@ void TSA_NO_TSA mmap_lock(void);
 void TSA_NO_TSA mmap_unlock(void);
 bool have_mmap_lock(void);
 
+static inline void mmap_unlock_guard(void *unused)
+{
+    mmap_unlock();
+}
+
+#define WITH_MMAP_LOCK_GUARD()                                            \
+    for (int _mmap_lock_iter __attribute__((cleanup(mmap_unlock_guard)))  \
+         = (mmap_lock(), 0); _mmap_lock_iter == 0; _mmap_lock_iter = 1)
+
 /**
  * adjust_signal_pc:
  * @pc: raw pc from the host signal ucontext_t.
@@ -683,6 +692,7 @@ G_NORETURN void cpu_loop_exit_sigbus(CPUState *cpu, target_ulong addr,
 #else
 static inline void mmap_lock(void) {}
 static inline void mmap_unlock(void) {}
+#define WITH_MMAP_LOCK_GUARD()
 
 void tlb_reset_dirty(CPUState *cpu, ram_addr_t start1, ram_addr_t length);
 void tlb_set_dirty(CPUState *cpu, vaddr addr);
diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
index aca8764356..74ed00b9fe 100644
--- a/bsd-user/mmap.c
+++ b/bsd-user/mmap.c
@@ -32,6 +32,7 @@ void mmap_lock(void)
 
 void mmap_unlock(void)
 {
+    assert(mmap_lock_count > 0);
     if (--mmap_lock_count == 0) {
         pthread_mutex_unlock(&mmap_mutex);
     }
diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 44b53bd446..a5dfb56545 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -36,6 +36,7 @@ void mmap_lock(void)
 
 void mmap_unlock(void)
 {
+    assert(mmap_lock_count > 0);
     if (--mmap_lock_count == 0) {
         pthread_mutex_unlock(&mmap_mutex);
     }
-- 
2.34.1

On Sat, 22 Jul 2023 at 12:35, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/exec/exec-all.h | 10 ++++++++++
>  bsd-user/mmap.c         |  1 +
>  linux-user/mmap.c       |  1 +
>  3 files changed, 12 insertions(+)
>
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index 5fa0687cd2..d02517e95f 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -629,6 +629,15 @@ void TSA_NO_TSA mmap_lock(void);
>  void TSA_NO_TSA mmap_unlock(void);
>  bool have_mmap_lock(void);
>
> +static inline void mmap_unlock_guard(void *unused)
> +{
> +    mmap_unlock();
> +}
> +
> +#define WITH_MMAP_LOCK_GUARD()                                            \
> +    for (int _mmap_lock_iter __attribute__((cleanup(mmap_unlock_guard)))  \
> +         = (mmap_lock(), 0); _mmap_lock_iter == 0; _mmap_lock_iter = 1)

All our other WITH_FOO macros seem to use g_autoptr rather than
a raw attribute((cleanup)); is it worth being consistent?
(This one also doesn't allow nested uses, I think.)

Either way
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

since it would be nice to fix this for the next rc.

thanks
-- PMM

On 7/23/23 15:18, Peter Maydell wrote:
> On Sat, 22 Jul 2023 at 12:35, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   include/exec/exec-all.h | 10 ++++++++++
>>   bsd-user/mmap.c         |  1 +
>>   linux-user/mmap.c       |  1 +
>>   3 files changed, 12 insertions(+)
>>
>> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
>> index 5fa0687cd2..d02517e95f 100644
>> --- a/include/exec/exec-all.h
>> +++ b/include/exec/exec-all.h
>> @@ -629,6 +629,15 @@ void TSA_NO_TSA mmap_lock(void);
>>   void TSA_NO_TSA mmap_unlock(void);
>>   bool have_mmap_lock(void);
>>
>> +static inline void mmap_unlock_guard(void *unused)
>> +{
>> +    mmap_unlock();
>> +}
>> +
>> +#define WITH_MMAP_LOCK_GUARD()                                            \
>> +    for (int _mmap_lock_iter __attribute__((cleanup(mmap_unlock_guard)))  \
>> +         = (mmap_lock(), 0); _mmap_lock_iter == 0; _mmap_lock_iter = 1)
> 
> All our other WITH_FOO macros seem to use g_autoptr rather than
> a raw attribute((cleanup)); is it worth being consistent?

I didn't think it worthwhile, no, since that requires even more boilerplate.

> (This one also doesn't allow nested uses, I think.)

It does, since each variable will shadow the next within each context.


r~